Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding WFS-T editing to GeoMoose #620

Merged
merged 22 commits into from
Mar 17, 2021

Conversation

theduckylittle
Copy link
Member

Big honking MR to add WFS-T editing capabilities to GeoMoose.

@theduckylittle theduckylittle changed the title Adding WFS-T editing to GeoMoose WIP: Adding WFS-T editing to GeoMoose Dec 29, 2020
@brentfraser brentfraser mentioned this pull request Jan 28, 2021
Base automatically changed from master to main February 5, 2021 21:15
@theduckylittle theduckylittle force-pushed the editing-work branch 2 times, most recently from 1cc047b to ea80452 Compare February 8, 2021 02:11
let queryFeature = util.featureToJson(evt.feature);
const mapProjection = this.map.getView().getProjection();

if (querySource.config['buffer-point']) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@klassenjs and @brentfraser - Thoughts on this pattern for querying points/lines layers?

Copy link
Contributor

@brentfraser brentfraser Feb 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related to #392, #536, #601. It's an OK fix. But someone should do some research on the OGC protocols to see if there is something in the protocol to do a spatial query by point geom.

@brentfraser
Copy link
Contributor

brentfraser commented Feb 27, 2021

Issue: Delete -> click on empty area (e.g. no polygon) shows "Remove ..." dialog (and clicking Okay shows error in console). No dialog should be shown.

@brentfraser
Copy link
Contributor

brentfraser commented Feb 27, 2021

Issue: Should the edit (and other?) tools be hidden when layer is turned off/disabled/not shown? Currently the legend is not shown in that case.

@brentfraser
Copy link
Contributor

Issue: If the editable layer is accessed through the "Favourites" tab, no "Edit feature properties" dialog is shown.

@brentfraser
Copy link
Contributor

Issue: Map tools (zoom in/out, end drawing buttons) obscure menu drop down choices. Change menu text from "Zoom to Full Extent" to "Full Extent", This move the other menu items slightly to the left. Start a feature edit, then click search and observe how the menu options are obscured by the Map tools.

image

@brentfraser
Copy link
Contributor

brentfraser commented Mar 3, 2021

Issue: When the WFS-T server is TinyOWS, and an attempt is made to edit properties on a features property values when they are all null, the fetch in wfs.js:171 fails:

               jsonFeature.properties.boundedBy = feature.getGeometry().getExtent();

because jsonFeature.properties is null (a little unexpected).

The fix is to test for null, and make the value an object:

                  if (jsonFeature.properties == null) {
                      jsonFeature.properties = [];
                  }
                  jsonFeature.properties.boundedBy = feature.getGeometry().getExtent();

or something similar.

GeoServer appears to include a "boundedBy" property by default, but TinyOWS does not.

@theduckylittle
Copy link
Member Author

Issue: Delete -> click on empty area (e.g. no polygon) shows "Remove ..." dialog (and clicking Okay shows error in console). No dialog should be shown.

Fixed. Wrapped the entire block in the feature-length check.

Should the edit (and other?) tools be hidden when layer is turned off/disabled/not shown? Currently the legend is not shown in that case.

This really isn't an "issue" we could hide the tools. My initial thought is to keep them visible since they indicate the capabilities of the layer before it's turned on.

Issue: If the editable layer is accessed through the "Favourites" tab, no "Edit feature properties" dialog is shown.

Moved the Modals' definition to the map from the catalogs.

Map tools (zoom in/out, end drawing buttons) obscure menu drop down choices. Change menu text from "Zoom to Full Extent" to "Full Extent", This move the other menu items slightly to the left. Start a feature edit, then click search and observe how the menu options are obscured by the Map tools.

  • Added some more Z to the drawers in the toolbar.
  • Renamed the Zoom to Full Extent tool.

boundedBy

Added a safer mixin to prevent errors.

@brentfraser
Copy link
Contributor

Should the edit (and other?) tools be hidden when layer is turned off/disabled/not shown? Currently the legend is not shown in that case.

This really isn't an "issue" we could hide the tools. My initial thought is to keep them visible since they indicate the capabilities of the layer before it's turned on.

Currently I put the editable layers in a separate group to indicate which are editable since that ability is so important to the user.

@brentfraser
Copy link
Contributor

@theduckylittle The latest commits are awesome! I've tested my "Issues" and all the bugs are fixed. As for the issue/opinion of "hide edit tools", having a clean looking catalog/layer list is important (as is having an understandable UI). One solution is to have an edit icon at the same level of the "refresh" and "metadata" icons on the layer name:

image

and while we're at it, move the "Toggle legend visibility" icon up there too.

@theduckylittle
Copy link
Member Author

@theduckylittle The latest commits are awesome! I've tested my "Issues" and all the bugs are fixed. As for the issue/opinion of "hide edit tools", having a clean looking catalog/layer list is important (as is having an understandable UI). One solution is to have an edit icon at the same level of the "refresh" and "metadata" icons on the layer name:

image

and while we're at it, move the "Toggle legend visibility" icon up there too.

I'm willing to open up the discussion on this. There are good/bad reasons for this and I'm willing to talk about them but not in this already monster sized pull request.

@brentfraser
Copy link
Contributor

@theduckylittle A discussion on the layer controls would be great!

@theduckylittle theduckylittle changed the title WIP: Adding WFS-T editing to GeoMoose Adding WFS-T editing to GeoMoose Mar 15, 2021
@theduckylittle
Copy link
Member Author

@klassenjs - I believe this is ready for code review.

@brentfraser has done a very thorough job of testing the functionality. I suspect any more bugs will be corner cases are minor oversights that can be handled in a follow up PR.

Commits can/will be squashed appropriately after review.

I suspect it's okay for docs to come in as a separate PR. I think Brent has those in the works.

Copy link
Member

@klassenjs klassenjs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, it might be worth bumping the version in package.json before the merge. No way this is a precursor to 3.7.1. Not sure if we want to call this 3.8 or 4.0.

examples/desktop/app.js Outdated Show resolved Hide resolved
tests/gm3/reducers/editor.test.js Show resolved Hide resolved
@klassenjs
Copy link
Member

klassenjs commented Mar 16, 2021

Drawing and markup->Draw Point
Tries to scroll map instead of adding a point.

Actually seem to get locked in scroll at end of drawing any feature on Drawing and Markup (with mouse button up)

Associated with the following in the console

Uncaught TypeError: i.queryAs is null
    value http://localhost/GeoMoose/gm3/examples/geomoose/dist/geomoose.min.js:27
    dispatchEvent http://localhost/GeoMoose/gm3/examples/geomoose/dist/geomoose.min.js:27
    finishDrawing http://localhost/GeoMoose/gm3/examples/geomoose/dist/geomoose.min.js:38
    handleUpEvent http://localhost/GeoMoose/gm3/examples/geomoose/dist/geomoose.min.js:38
    handleEvent http://localhost/GeoMoose/gm3/examples/geomoose/dist/geomoose.min.js:27
    handleEvent http://localhost/GeoMoose/gm3/examples/geomoose/dist/geomoose.min.js:38
    handleMapBrowserEvent http://localhost/GeoMoose/gm3/examples/geomoose/dist/geomoose.min.js:38
    dispatchEvent http://localhost/GeoMoose/gm3/examples/geomoose/dist/geomoose.min.js:27
    handlePointerUp_ http://localhost/GeoMoose/gm3/examples/geomoose/dist/geomoose.min.js:83

@klassenjs
Copy link
Member

The CSS changes are mostly subtle but it looks way nicer and feels more intuitive than before.

@klassenjs
Copy link
Member

I'm still working on a Geoserver install.

1. Fix bug with joining "?" urls

- Adds new util function to centralize cleanup logic.
- Fixes vector map-source issue when there are trailing question marks.

2. Add built-in fixed editing layer.

3. Convert map controls to new react components
4. add keybindings for edit operations
1. Add affordances to selected tools in the catalog.
2. Add signifiers to selected services in the toolbar.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants