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

Make geometries non-movable in the Exploration feature #936

Merged

Conversation

dzole0311
Copy link
Collaborator

Related Ticket:

Addressing #934

Description of Changes

Two changes made, both affecting only the new map component in the Exploration page:

  1. Geometries added from presets are non-draggable & non-editable
  2. User-drawn & uploaded geometries are also non-draggable, but their vertices can be moved

Notes & Questions About Changes

  • Added a new package in the project to initiate StaticMode (mode with no interactions on polygons): https://github.com/mapbox/mapbox-gl-draw-static-mode
  • Override the dragMove and the dragFeature methods for the simple_select and direct_select
  • We might intend to do more complex interactions in the future that need moving / dragging, so aside from custom modes, an alternative could be some kind of filtering in the original mode itself based on feature props? Ie. if a feature has a prop isPreset, then it should be handled differently etc.
  • This will probably affect the Data Analysis page as well, once 827 is merged

Validation / Testing

  1. In E&A, select a preset -> it should be static so that no vertices are visible nor it can be dragged
  2. On the same page, draw a polygon -> the polygons vertices should be draggable, but the polygon itself should be non-movable
  3. The same behavior as step 2 applies to uploaded files

Copy link

netlify bot commented May 6, 2024

Deploy Preview for veda-ui ready!

Name Link
🔨 Latest commit 1d61f20
🔍 Latest deploy log https://app.netlify.com/sites/veda-ui/deploys/6638c45857e9d50008758a7d
😎 Deploy Preview https://deploy-preview-936--veda-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@dzole0311 dzole0311 requested a review from faustoperez May 6, 2024 11:44
@dzole0311 dzole0311 linked an issue May 6, 2024 that may be closed by this pull request
1 task
Copy link

@faustoperez faustoperez left a comment

Choose a reason for hiding this comment

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

I confirm the presets and the custom geometries are working as expected. Great job @dzole0311 🙌

modes: {
...MapboxDraw.modes,
simple_static: StaticMode,
simple_select: customSimpleSelect,
Copy link
Collaborator

Choose a reason for hiding this comment

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

At this point, what is the difference between simple_static and simple_select mode?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I should have really named this only static instead of simple_static, but it's basically two different modes. simple_static disables all user interactions (including dragging, and prevents vertices from showing up per polygon) because that's the behavior we want to try out for the presets. The simple_select mode is almost the same as before, aside from the dragging of the feature as a whole being disabled (but vertices are still shown and can be interacted with).

So simple_static is only for the presets at the moment, while the simple_select is for uploaded assets or hand-drawn features.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@hanbyul-here
Copy link
Collaborator

Thanks for a clean and concise solution 👏 I left a question to understand more clearly about when to use simple_static and simple_select.

Copy link
Collaborator

@sandrahoang686 sandrahoang686 left a comment

Choose a reason for hiding this comment

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

Congrats on your first couple of PRs! lgtm

@j08lue
Copy link
Contributor

j08lue commented May 6, 2024

Got the tiniest of nitpicks - even though user-drawn geometries are not draggable (as intended), the mouse cursor still seems to indicate that it is (four-headed arrow ✥ ish).

Not a blocker, though - please go ahead and merge this as-is!

@hanbyul-here hanbyul-here merged commit cf11f1c into main May 6, 2024
8 checks passed
@hanbyul-here hanbyul-here deleted the feature/make-geometries-non-movable-on-exploration branch May 6, 2024 19:02
hanbyul-here added a commit that referenced this pull request May 6, 2024
### Features

* Floating Data Catalog Filters Sidebar Follow-up and starting some
cleaning and modularizing in
#930
* Make geometries non-movable in the Exploration feature in
#936
@hanbyul-here
Copy link
Collaborator

hanbyul-here commented May 6, 2024

My apologies that we missed this one bug: US-GHG-Center/veda-config-ghg#372 (review)

We can find a fix for this problem in many ways, but the foundational problem is that the state of MapboxDraw and React are not syncing (This problem is stated in this ticket: #710 (comment)) - We assume that any newly added feature is selected and mark it through atom but the feature added through preset is not selected in MapboxGL Draw.

Now we have 3 ways of adding AOIs. We should walk-through what is expected for each mean and match the behavior between MapboxGLDraw and React.

@j08lue
Copy link
Contributor

j08lue commented May 6, 2024

My apologies that we missed this one bug

That is on me for rushing this through. 🙇

Let us see what the best resolution is - short-term and longer-term.

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.

On Exploration make geometries non-movable
5 participants