-
Notifications
You must be signed in to change notification settings - Fork 26
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
Allow more map interactions and refactor map value datastructures #4953
Allow more map interactions and refactor map value datastructures #4953
Conversation
4aaecd1
to
45f8fdc
Compare
45f8fdc
to
9653b84
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4953 +/- ##
==========================================
+ Coverage 96.63% 96.66% +0.03%
==========================================
Files 761 762 +1
Lines 26004 26041 +37
Branches 3395 3394 -1
==========================================
+ Hits 25129 25173 +44
+ Misses 610 606 -4
+ Partials 265 262 -3 ☔ View full report in Codecov by Sentry. |
9653b84
to
3438a77
Compare
dcee0e7
to
42885c6
Compare
d8451a5
to
ba5f7a2
Compare
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.
Might be a good idea to disable prettier on JSON files, I had to do the same for markdown and HTML too 😉
type Coordinates = tuple[float, float] | ||
|
||
|
||
class PointGeometry(TypedDict): |
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.
wouldn't it make more sense to define these types in src/openforms/api/geojson.py
too? Ultimately these are the types that match the serializers.
Otherwise I'd say to put them in openforms.typing
.
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.
I thought that these should remain near the MapFormatter
, seeing how these types are only used for this class.
And also to keep the code uniform to the AddressNLFormatter
/ AddressValue
implementation.
But sure, i can move them
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.
I've moved most of these types to src/openforms/api/geojson.py
. I've kept a MapValue
type in the src/openforms/formio/formatters/custom.py
, to specify the value for the format
function
src/openforms/registrations/contrib/objects_api/submission_registration.py
Outdated
Show resolved
Hide resolved
src/openforms/registrations/contrib/objects_api/submission_registration.py
Outdated
Show resolved
Hide resolved
Ah yeah, probably best to add those to the .prettierignore 🙂 (just to solve it project wide) |
Let's enable prettier on json files inside |
For the map interaction changes, we need to save the map data in a different format. This is to allow us to differentiate between point, line or polygon map information. At the moment we only need the geometry information of the geoJson, so thats what we will store. The other geoJson information, `type` and `properties`, are at the moment not used. In addition, the backend registrations (objects API, ZGW APIs and StUF-ZDS) also only need the geometry information.
The backend registration plugins (Objects API, ZGW APIs and StUF-ZDS) now support point, line and polygon geojson geometries.
Changed the map data from coordinates to geoJson geometry values.
Map component that don't have interactions configuration, will receive the default config when importing using the migration_converters
The form designer configuration for the map component has changed a bit. So are the map specific settings moved to the 'Map settings' tab. Also, the label for initial lat/lng settings has been changed to 'Initial focus'
…tions The formatter and camunda tests now cover all map data formats: Point, Polygon and LineString
165c370
to
308f1c4
Compare
@@ -21,10 +23,16 @@ def format(self, component: Component, value: str) -> str: | |||
return f"{fmt_date(parsed_value)} {fmt_time(parsed_value, 'H:i')}" | |||
|
|||
|
|||
type MapValue = PointGeometry | LineStringGeometry | PolygonGeometry |
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.
this is the ideal compromise - it keeps the stuff that's not specific to map components in a generic place while still refining it sufficiently within the map component context
Closes #2177
Closes #2178
Changes
Changing the datatype of the map component from array to geoJson
Checklist
Check off the items that are completed or not relevant.
Impact on features
Release management
I have updated the translations assets (you do NOT need to provide translations)
./bin/makemessages_js.sh
./bin/compilemessages_js.sh
Dockerfile/scripts
./bin
folderCommit hygiene