-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: added format handler for different geometry types for jsonform #1276
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for eoxelements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@silvester-pari i think this brings great first implementation to support different types of geometries into the jsonform using the draw tools. There will be nuances that need to be solved, e.g. also to better support specialized STAC schemas, but i think as is it already provides great value and would propose to merge and continue improving in smaller increments afterwards. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, looks like a big addition! I am unsure about a few points however:
- the
*-editor
formats: I believe it doesn't make sense to offer this distinction, as the drawing of a bbox should always allow to edit the coordinates manually (via a number input) in order to fine-tune the drawing. I get that the current editor that comes with drawtools is not ideal for that (layout, and showing the entire feature content instead of just the coordinates), but here we maybe can think of a different solution (e.g. keep the coordinate input fields as per default json-editor, and just add the map additionally) - having to click the "draw" button in order to be able to start drawing: I feel like from a user perspective this is an unnecessary step and could cause quite some friction. I understand that this is the technical solution we have, but it's less than ideal from the user experience. Ideally the user comes to the section where the map appears (by default it should not be scroll-zoomable by the way) and can simply draw the corresponding shape
- changing of the current example schemas: it's probably not wise to change the current story schemas (as in the collection schema) just to showcase something that works quite differently; the current example schema is designed with STAC in mind and I wouldn't like to completely dismiss this (when we remove it now from the codebase, will it ever come back?). I would have hoped that the new functionality would be an addition that works with the existing schema, and not require a completely new one.
All in all, it feels a bit like a very much technology-driven solution, not necessarily a developer- and user-friendly one; maybe we can find compromises, offering less functionality but in a more user-friendly manner? 🤔
Maybe also the drawtools are not the ideal fit for the jsonform, and we maybe should just use the map and then some input fields generated by jsonform; but happy to do some more brainstorming and/or experimentation!
Thank you for your feedback! I would like to note that the implementation was intended with much developer experience and user experience in mind, I would like to clear and comment on some points:
That being said, we can simply revert the change in the collection and it would go back to how it was.
PS. the changes has been made after @silvester-pari asked in the daily about how would this PR be integrated with the collection schema, and it was intended to show case that.
I think this fulfills the requirement of it being UX friendly, but If you think that it would be better to start the drawing without pressing on the button, I would be happy to implement that.
|
Thank you @A-Behairi for the detailed answer! Here are my thoughts:
This was the previous schema:
For bounding boxes, I would imagine something like this:
Note a few things:
|
hi! it seems there is quite some definition work that was needed for this, as it is bringing in concepts needed in multiple places, i will try to recap things to see if we agree on it (i already had a quick catch up with Ahmed about these points).
Topic:
Additionally: |
Trying to integrate a new statistical api experiment i realized that geometries can be expected in at least a couple of different ways.
|
Thank you for all your inputs, find below a list of all the adjusted schemas that renders the drawtools. // enables drawing multiple boxes
// returns an array of extents
{
type: "array",
format: "bounding-boxes",
},
// enables drawing multiple boxes
// and returns a wkt formatted string
{
type: "wkt",
format: "bounding-boxes",
},
// enables drawing multiple boxes
// and returns a geojson object
{
type: "geojson",
format: "bounding-boxes",
},
// enables drawing a single box
// and returns the the extent of the bbox
{
type: "array",
format: "bounding-box",
},
// enables drawing a single box
// and returns a wkt formatted string
{
type: "wkt",
format: "bounding-box",
},
// enables drawing a single box
// and returns a geojson object
{
type: "geojson",
format: "bounding-box",
},
// enables drawing multiple polygons
// returns an array of the drawn features
{
type: "array",
format: "polygons",
},
// enables drawing multiple polygons
// returns a wkt formatted string
{
type: "wkt",
format: "polygons",
},
// enables drawing multiple polygons
// returns a geojson object
{
type: "geojson",
format: "polygons",
},
// enables drawing a single polygon
// returns the drawn feature object
{
type: "object",
format: "polygon",
},
// enables drawing a single polygon
// returns a wkt formatted string
{
type: "wkt",
format: "polygon",
},
// enables drawing a single polygon
// returns a wkt formatted string
{
type: "geojson",
format: "polygon",
},
// enables drawing multiple points
// returns an array of the points coordinates
{
type: "array",
format: "points",
},
// enables drawing multiple points
// returns a wkt formatted string
{
type: "wkt",
format: "points",
},
// enables drawing multiple points
// returns as a geojson object
{
type: "geojson",
format: "points",
},
// enables drawing a single point
// returns the coordinates of the point
{
type: "array",
format: "point",
},
// enables drawing a single point
// returns a wkt
{
type: "wkt",
format: "point",
},
// enables drawing a single point
// returns a geojson object
{
type: "geojson",
format: "point",
},
// enables selecting a feature from a layer
// the returned type can vary based on the provided `type` and `options.featureProperty`
// if no `optons.featureProperty` is provided the returned value is the feature object
{
format: "feature",
},
// enables selecting multiple features from a layer
// the returned value is an array of values that can vary based on
// `items.type` and `options.featureProperty`
// if no options.featureProperty is provided the returned value is an array of feature objects
// and the `items.type` should be set to "object"
{
type: "array",
format: "features",
},
// enables selecting multiple features from a layer
// the returned value is a wkt formatted string
{
type: "wkt",
format: "features",
},
// enables selecting multiple features from a layer
// the returned value is a geojson object
{
type: "geojson",
format: "features",
} supported options:
|
Hello @A-Behairi this is looking great, the type options are really helpful to support different endpoint capabilities. Any opinion on this would be very welcome, please let me know what you think. While writing this i realized i know of use cases where we will need to define a line, so if it is not too much effort we could use the opportunity to add this format also here, we could call it Examples for single select: {type: "geojson", format: "point"}
{type: "geojson", format: "polygon"} and {type: "geojson", format: "bounding-box"}
{type: "geojson", format: "line"}
|
Implemented changes
Screenshots/Videos
Checklist before requesting a review