Geometry editor - #15
Open
coret wants to merge 7 commits into
Open
Conversation
…form A geometry value rendered as its raw wkt, and entering one meant typing wkt into a textarea. Both now use a Leaflet map, built entirely from assets inside the module. Rendering. A geometry or geometric-coordinates value is displayed as a map with the geometry drawn on it. Geometric position is not: its origin is the top left corner of an image, so "4,52" means four pixels across and fifty-two down, and on a world map that lands in the Gulf of Guinea. Geometric coordinates store "x,y" rather than wkt, so that type converts through its existing getGeometryPoint() before handing anything to the map. Configuration travels in a data attribute rather than an inline script, so a resource with several geometries draws several maps and none of them needs a known id. Editing. Ctrl+Alt+M inside a geometry or geography field, or the "Select on map" button beside it, opens a map in Omeka's sidebar. What is drawn is written back into that same field as wkt and re-validated as if it had been typed, so nothing about how a value is stored changes. One shape per field: a second would have to be written as MULTIPOLYGON and friends, which this module's own validation rejects. Circles are not offered, having no wkt representation. An editor closed without drawing writes nothing, because reading a value in and writing it back out is not a round trip for every geometry. This closes the "select on map" item in the readme's todo list. Leaflet is loaded lazily in the resource form rather than with the page. The Mapping module puts its own copy on the same item edit form, a second one would replace window.L under it, and the order in which two modules append to headScript is not something either can control. Loading late means the editor can see what is already there and reuse it; Leaflet.draw 1.0.4 works against both copies. Layers are configured under the datatypegeometry key, defaults in config/module.config.php and overrides in Omeka's local.config.php. OpenStreetMap ships as the base layer so a stock installation draws something. No overlays ship: which historical maps are worth showing is a property of a collection, not of this module. A layer url is used exactly as written, so a caching proxy in front of a tile server is simply part of the url. The bundled libraries are recorded in asset/vendor/VERSIONS.md, with the traps met while assembling them: leaflet.fullscreen needs its UMD build, since 5.0.0 the default dist is an es module that throws in a script tag; Leaflet.draw's dist was trimmed to the files its css references; and Leaflet 1.9's plus-lighter tile blending is left alone here because these maps stay at integer zoom. .gitignore keeps ignoring asset/vendor except for these three, so the terraformer asset is still fetched by composer. tests/verify-wiring.php checks the wiring against a real installation. The maps themselves need a browser, so tests/browser holds two pages driving the editor: one for its own behaviour, one proving the Mapping module's Leaflet survives it.
The marker distinguished a local patch from the module around it. In the module's own history it distinguishes nothing, so it is noise. The comments themselves are kept: what they explain is still worth knowing. The hash comment style went with it, that having been part of the same convention; the rest of the codebase uses //. Comments only: no statement changes.
The panel was widened to 40% so a shape could be drawn without panning, but core couples that width to an offset: .sidebar is width:25% parked at left:100%, and .active slides it to left:75%. Widening one alone left the panel running from 75% to 115%, and .sidebar's own overflow-x:hidden cropped the overhang. What sits in the cropped strip is the layer switcher, in the map's top right corner, so it was invisible. Sets left:60% alongside width:40%, and drops the min-width, which broke the same arithmetic again between 641px and 950px. Both are scoped above core's 640px breakpoint, below which the sidebar is full width already. tests/browser/sidebar-layout.html covers it: it loads Omeka's own stylesheet, which the other two browser pages do not, because without it the sidebar has no geometry and a map overflowing the panel looks fine. It measures against the viewport as well as against the panel — the switcher was positioned correctly relative to its parent throughout — and confirms with elementFromPoint that the control is reachable rather than merely placed. Appending ?bug=1 restores the mistake, and three assertions fail.
The public map got the fullscreen control when this work replaced the stylesheet-without-its-script arrangement that had styled a button no map ever created. The editor never did: it lazy-loads what it needs, and the list was Leaflet and Leaflet.draw. Passing fullscreenControl to a map whose plugin was never loaded is ignored in silence, which is exactly the failure this work set out to remove, reproduced in the other half of it. The plugin now loads alongside Leaflet.draw, under the same rule as the rest: only if it is missing. On the item edit form it usually is not. Mapping loads leaflet.fullscreen 2.4.0 there, an ordinary script rather than the es module later majors ship, registering the same L.Control.FullScreen under the same leaflet-control-zoom-fullscreen class, so the editor takes it and adds nothing. collision.html now preloads all three of the files Mapping's item form loads, rather than two, and asserts the sharper thing: that the editor pulls nothing whatsoever from this module's asset/vendor when the page already has the stack. Its old assertion counted script[src*="leaflet"], which a plugin path also matches, so it read this commit's extra file as a second copy of Leaflet.
Leaflet 1.9 composites tiles with mix-blend-mode: plus-lighter, to hide the dark seam a tile shows while it fades in. On a base layer as light as OpenStreetMap it does the opposite: wherever two tiles overlap by a fraction of a pixel, the sum clips to white and the edge draws as a white line. Both maps are put back to normal. The overlap does not come from Leaflet. These maps sit at integer zoom and Leaflet does not scale their tiles — asset/vendor/VERSIONS.md said so and concluded no override was needed, which was wrong, because the browser scales them too. At 110% page zoom, or on a display with fractional scaling, a 256px tile is laid out across 281.6 device pixels. That is how the bug was recognised from the screenshot it was reported with: the white grid was spaced 281 to 282 pixels, not 256. Measured across the map interior at scale 1.1, on a flat tile the colour of OpenStreetMap's land: 1006 pure-white pixels with plus-lighter, none with normal. On real tiles, seam columns 281px apart and some 35 grey levels above their neighbours, and none after. Normal is never worse at any scale factor tested — 1, 1.1, 1.25, 1.5 — and strictly better at 1.1 and 1.5. The trade is a tile may show its dark edge while it fades in. The selector names both .datatype-geometry-map and .geometry-map-canvas, the public map and the editor's, at a specificity that beats Leaflet's own without depending on stylesheet order — Mapping may have loaded its Leaflet after this module's. tests/browser/tile-seams.html keeps the measurement, with toggles for putting plus-lighter back and for swapping the flat tile for the real base layer. Nothing shows at scale 1, so render it at 1.1.
"Select on map" becomes "Use geometry editor", and the panel heading "Draw the geometry" becomes "Geometry editor". Both are translatable strings, so the entries in js_translate_strings move with them, as does the pair verify-wiring.php checks for. The readme's todo list keeps its original wording: that line is upstream's own request, and rewording it would misrepresent what was asked for rather than record that it is now done. Also closes an html comment in collision.html that a previous commit ended one line early, leaving five lines of prose loose in the document head.
The rename split "Use geometry editor" across a line break, which reads fine but cannot be grepped, and pushed two lines past the file's margin.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I added a Leaflet based geometry editor so a user doesn't not have to enter/edit WKT.
Note: I've tested the module and use it a lot with the Gouda Timemachine, but I didn't change the version number of the Omeka S module.