Edits #127
Edits #127
Conversation
Putting modified data in a new browser window is about the only thing a "save" button can do in a purely client-side framework; it's then up to the user to put it where it can do some good. Problems: *) button needs better placement *) button should be suppressed if editing not configured *) generated output is not pretty-printed, and fugly as hell. Still, going for proof of concept here, and this is it.
Turns out this was *not* hard. OK, then.
Furthermore, dragging it alters the stored geoJson, and is correctly reflected on export, all managed by the edit_state component (which is listening for "selectFeature" and "selectedFeatureMoved" events). Works when tested manually, and verifies the general theory of edits for other stuff (and how I expect edit state to be bound to the UI, at least in general terms). Automated tests tbd.
These are all component unit tests, and get awkwardly deep in leaflet internals when looking at drag state, but better than nothing.
Right now, I'm just being consistent; this will matter if these events ever come from elsewhere (e.g., updated on changes to address properties).
Now to integrate a JSON editor so we can start sending them...
Using Jeremy Dorn's json editor, replacing the contents of the info window. Needs a little glue code to connect up to the edit state object. More seriously, needs to have either scrollbars, much smaller fonts, or both, to get adequate space.
Late fixup to an editor config screwup on my part.
Still work in progress -- needs styling and tests.
|
This looks really cool. You're right that is still needs some formatting/UX love. Check out the google maps/mapbox editors for inspiration. |
Also set max height for the pane itself, so it gets a scroll bar.
If nothing else, this cuts down on spurious widgetry from the JSON editor.
This does involve storing its font files. Sigh...
Otherwise, if the user scrolled down, they wind up in the middle of the new feature's properties, which is almost certainly not what they want to be first looking at.
Turns out this is just a change in the JSON schema we give the editor.
|
Properties editor is much improved now: if you give the JSON editor a more detailed schema about what to do with properties, and a little more space, it winds up looking much better. (In part because it's no longer cluttered up with the elaborate widgetry that users could use to turn every darn string into an array if they so chose.) Next up: double-click on the map to create a feature. (Already coded, but needs tests.) And at some point I get to figure out what Travis is gagging on. (It claims syntax errors, but tests do pass on my laptop.) |
This is actually done in two stages: the double-click gives you a popup in which you enter the name of the feature you're creating (well, the map's preview feature, whatever that is); submitting the form actually creates the feature, and gives you the full feature editor. To minimize mousing-around, the popup form automatically gets keyboard focus when created. The intent of this dance is to make a stray double-click easy to undo. Internally, what happens is that on submission, the popup form handler sends a 'newFeature' event, which anything maintaining its own copy of the data ought to handle. The edit-state does that by adding the feature to its data. The map adds a new marker. (The map is also, currently, what sends the event, but you can imagine cases where it wouldn't be.)
The only slightly messy thing is handling JSONEditor, but the ugliness was already there; this just makes it explicit.
Might want to do a rebase and a new PR before this gets pulled into mainline (if they don't just squash on merge), but for now, if it's there, it might as well be current.
The JSON Editor marks these up as <h3>s; we style 'em here to have (pretty much) the same size and weight as the normal labels it uses for simpler properties.
Due to a leaflet bug, we need to disable dragging *before* calling setIcon. http://stackoverflow.com/questions/22671733/why-doesnt-marker-dragging-disable-work
For this purpose, map.js uses a map indexed by the [lng, lat] coordinate arrays from the feature geometries. And the hash lookups for those, in turn, do depend on the coordinate values. So, at least as a stopgap, we now re-record the feature-to-marker mapping in this map after drag-end.
Leaflet sets timeouts internally. These can fire after the test that created the map is over, and the map is no longer present in the DOM. That doesn't always go well. This patch adds an afterEach to the map specs, which effectively defangs a timeout for which it can go rather badly -- throwing an exception in the timeout, which causes an early abort from the whole test run when the exception goes uncaught. See comment in the patch for details.
Mentions that data isn't actually gone until export, and that "restore" is available to undo a (mark-for-)deletion.
This is meant to be generic, so use terms less tied to the sample data.
This time for sure! (Storing features in the memoized-markers hash is not usually a particularly happy thing...)
To avoid thrashing completely, we do this in a timeout, ten seconds after things go quiet.
Some of this text was formerly part of a TODO document.
|
Hi Robert,
|
The edit-mode and non-edit-mode variants act different enough that there's no obvious reason to have them implemented by the same piece of code.
Specifically, this gives the edit-info pane a "revert" button which restores the feature's position, properties, and delete status to whatever they were on first load. The edit-state component is in charge of managing undo status, which it does by stuffing stuff into the "undoInfo" of features (a member which no one else is supposed to look at). Other components are made aware of the undo status of the selected feature (if they care) by listening for the 'selectedFeatureUndoStatus' event, which carries a boolean value (true if "revert" will do something, false if the values are unchanged). Other components can request an undo by sending the "requestUndo" event, which causes the edit state to fish out the original values, and send "selectedFeaturePropsChanged", "selectedFeatureMoved", and undelete events to restore the original values. (As a consequence, the infoedits component is now listening for prop-change events, so that it shows the reverted values when a revert occurs!) This protocol is designed so that if we want to go to an actual per-event undo stack, nothing needs to change outside of the edit state (though we might want to relabel the revert button!). For map-wide undo state, the protocol would have to change somewhat, but then again, so would the UI; it would no longer be appropriate to have the undo-triggering button on a control tied to a particular feature.
|
In edit mode only, this creates a second submit button on the geosearch form (if that is enabled!), which initiates the process of creating a new feature at the location, if the geosearch result shows a location. This is an alternative to double-clicking on the map, which may be quicker in some cases -- users will have to type the address anyway, and this way, they can be sure of the exact spot. (Or, well, at least as sure as they are of the geolocation server.)
Conflicts: src/script.js src/ui/map.js Also, had to make sure that the marker being edited was explicitly added to the map, and not buried in a cluster. (There are still some glitches to clean up when a point is selected from the new list widget; you need to re-select the feature before it becomes draggable.)
... that is, the button that shows up in edit mode to let you add a point at a typed-in address.
|
BTW, following up belatedly on some in-person discussion with Paul: right now, in edit mode, there are two different lists of attributes. But they're tricky to merge because they serve different purposes. The "properties" list, despite the name, is actually a list of presentation lines: in the sample config, for example, the "address" property is actually listed twice, because it's presented two different ways: presented as text ('address'), and as a directions link ('{"name": "address", ... "directions": true}'). It also just omits features which aren't being presented at all. The JSON schema used by the edit widget, by constrast, is a description of the data; it can include each property only once, and if any are omitted, things get ugly. Still looking for an elegant way to merge the two... |
Conflicts: index.html karma.conf.js src/data/facet.js src/script.js src/ui/map.js test/spec/ui/map_spec.js
This involves adapting it to new data structures, with features having explicit ids, and those being used to index the feature-to-layer map in map.js
'script.js' is used to load code into live browser environments only; karma is configured differently. Which means that script.js itself winds up with no test coverage...
This can happen when creating new features in edit mode; values start out undefined ... and undefined.toLowerCase() is not a fun time.
Still some glitches on creation, though all tests pass. Conflicts: src/ui/map.js
This prepends them, rather than putting them in sorted position, but it's something...
Should check that other IDs are preserved. And ideally have a better check than this syntactic one about whether IDs are internally assigned.
I'm actually requesting review of some code for editing the map within finda. This does not yet close #84 (or shouldn't!), but there's enough here, I think, to demonstrate proof of concept, and it's probably time for someone else to give the code a good hard look for maintainability and functionality.
Here's what works: if you put '"edit_mode": true' in the config, you get an "Export Data" button in the navbar, and when you select a point, you can reposition it (by dragging the marker), and edit the properties (using Jeremy Dorn's JSON Editor, which replaces the contents of the info pane). The "Export Data" button puts the edited GeoJSON in a separate browser window, from where you can "Save As" and stick it where you like; this may be more convenient than asking users to paste it out of a text box.
Here's what's not there yet: First off, the JSON Editor control really doesn't have enough room in the info box. It needs more width, shrunk fonts, or both. There is not yet a way to create a new point (scratch plan: double-click on the map) or delete one. There's also no form of undo; actually releasing an editor without this is against my religion. (Scratch plan: on first change, record initial position and properties of the point; after that, offer a "revert" button. Also, "delete" would just set a "deleted" flag and leave the point visible, with the marker's opacity set to 0.5, and available for "undelete"; points with the "deleted" flag set would be skipped on an export. All subject to change, but I don't think this is all that much code.)
A few more details: most of the new code is guarded by "if (editMode)" hooks of one kind or another, so display behavior with edit mode off should be unchanged.
There's a demo at http://www.panix.com/~rst/finda/ if you'd like to see it in action.
Have fun, and hope this is useful.