-
Notifications
You must be signed in to change notification settings - Fork 22
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: [DHIS2-13237] Enrollment coordinates in enrollment widget #3141
Conversation
I see a map, that's good 🙂 Tried to register a new TEI and enroll, but the marker is not placed in the right spot. Maybe a mixup of Latitude and Longitude? (or are we been inconsistent here somehow) Also, we should be able to update or set a new coordinate/polygon somehow, started a thread on slack for this since the design is incomplete it seems. |
🚀 Deployed on https://deploy-preview-3141--dhis2-capture.netlify.app |
src/core_modules/capture-core/components/MapCoordinates/MapCoordinatesModal.js
Outdated
Show resolved
Hide resolved
src/core_modules/capture-core/components/MapCoordinates/MapCoordinatesModal.js
Outdated
Show resolved
Hide resolved
src/core_modules/capture-core/components/MapCoordinates/mapCoordinates.types.js
Outdated
Show resolved
Hide resolved
...odules/capture-core/components/WidgetEnrollment/Actions/AddLocation/AddLocation.component.js
Outdated
Show resolved
Hide resolved
...odules/capture-core/components/WidgetEnrollment/Actions/AddLocation/AddLocation.component.js
Outdated
Show resolved
Hide resolved
src/core_modules/capture-core/components/WidgetEnrollment/MapModal/Polygon/Polygon.component.js
Outdated
Show resolved
Hide resolved
Hey @superskip, I've rollback the disabled buttons logic and implemented the same design we currently have when the polygon is created/edited/deleted when enrolling a TEI. This fixes the original issue you reported:
IMHO I would rather merge the enrollment map with the same design as the existing one (just as the Jira ticket description mentions). Afterward, if it needs improvements, we can focus on changing all the maps at the same time, not just the enrollment widget map. Let me know if you strongly disagree or find any difference between how the polygons modal works in both places. Thank you for the review! |
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.
The inviting blue button is definitely evil now.. I don't think this feels very releasable at the moment, even if the profile widget does the same. If we're in a hurry with releasing this, maybe we should make it look like a useless feature like in the profile widget, by not showing the polygon any more as soon as the page gets reloaded.
If we can make a common map modal component for both widgets, then I agree it would make sense to improve both simultaneously. I also imagine much can be done by improving the enrollment widget first, if one focuses on making the component reusable.
This issue wasn't yours from the start, so we might consider passing it on to me for a while if you're tired of working with it 😛
onUpdate={updateMutation} | ||
enrollment={enrollment} | ||
/> | ||
{isOpenMap && ( |
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.
Ah, nice solution :)
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.
@superskip gave me an overview of the problems he saw. Based on this I suggest the following:
- When opening the modal, before any changes have been made, don't display the 'Set area' button at all. We should keep the cancel button, but change the text to 'Close'.
- When starting to draw an area, display 'Close without saving' (same behaviour as current 'Cancel') and 'Set area'. 'Set area' should be disabled (also show a tooltip, 'finish drawing before saving')
- After an area has been drawn, display the buttons we have today (but replace 'Cancel' button text with 'Close without saving')
- Clicking 'cancel' in the drawing should get you back to the previous state (If no changes have been made earlier, we will only display 'Close' for example)
- Remove the option to edit the current polygon (the button that says "edit layers"). If you need to make changes, you will have to draw a new one. We need a proper design here ( https://dhis2.atlassian.net/browse/DHIS2-15970), so we will implement in different ticket.
Also, we should consolidate the map code at a later point: https://dhis2.atlassian.net/browse/TECH-1660
Let's talk if you have questions / concerns
Hey @JoakimSM, |
Much better! Sometimes when I'm deleting (see video), it doesn't actually get deleted. I think the problem always occurs when: First editing an existing area and saving (set area), then clicking the minimap again and deleting (no browser refresh in between). Skjermopptak.2023-11-10.kl.15.24.49.movThis is hopefully the last hurdle. |
Hey @JoakimSM, |
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.
🎉
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.
Tested successfully on 2.41,2.40.3,2.39.4,2.38.6 versions
# [100.45.0](v100.44.7...v100.45.0) (2023-11-20) ### Features * [DHIS2-13237] Enrollment coordinates in enrollment widget ([#3141](#3141)) ([2f2e52c](2f2e52c))
🎉 This PR is included in version 100.45.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
DHIS2-13237
Tech summary
AddLocation.component.js
MapModal
Add Coordinates
orAdd Area
based on thegeometryType
MiniMap.component
defaultValues
toMapModal
MapModal.container.js
onSetCoordinates
uses theuseUpdateEnrollment
hook to update the geometryCoordinates.component.js
renderLatitude
andrenderLongitude
display the coordinates of the point under the mapisValidCoordinate
convertCoordinatesToServer
Polygon.component.js
convertPolygonToServer
DeleteControl.component