-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Maps] Design updates for draw shape mode and timeslider #103493
Conversation
Pinging @elastic/kibana-gis (Team:Geo) |
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.
LGTM - looks great
code review
@miukimiu
Why does the timeslider "wobble" when it is first shown? |
As per my description:
It's an animation. I was thinking like "I'm here, in case you didn't notice!". But if it's too much I can remove it. 😛 @gchaps and @kmartastic for the labels I think we should be more in sync with this description: In one place we call it "add shapes" and in other places "features". I think "features" is not a very descriptive name. So should it be "Stop shapes editing"? And inside the popover "Edit shapes" instead of "Edit features"? |
@elasticmachine merge upstream |
@miukimiu |
@nreese yes. 👍🏽 |
I think its important to keep the word "feature". "Stop editing" is too vague. Are you stopping editing the layer settings or stopping editing the features of the layer. I like prefer "Exit feature editing" over "Stop feature editing" for what its worth. |
@kmartastic the timeslider animation was updated. @nreese the The video on top of the page was also updated with the new changes. |
@nreese Would you mind taking a look at the changes I introduced in the linked commit to show edit tools only for new vector layers? |
@aaronjcaldwell I think for beta we can move without the indication. |
sorry for closing. Not sure what happened. |
Discussed offline, but for documentation purposes: In the flow of adding a new, empty vector layer, we want to show the edit tools automatically. To be clear, this only happens once when the layer is added initially. After this, showing edit tools functions as you would expect for any eligible layer. |
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 think this is a pretty clean way to auto opening edit menu for a newly created draw layer. LGTM with minor feedback on names and some duplicated state.
x-pack/plugins/maps/public/connected_components/add_layer_panel/view.tsx
Outdated
Show resolved
Hide resolved
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.
lgtm w/ green CI! Thanks for the polish! 🎆 A few notes:
- Pushed up the changes we discussed: show edit tools when adding a new vector layer. Also made some additional follow-on changes requested in feedback
- Regenerated a jest test snapshot that had changed with the addition of a react property
- Added one more dispatched function to
cancelEditing
for consistency with similar functions
Review:
- tested locally in chrome
- code review
@elasticmachine merge upstream |
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.
Code review. LGTM. Small comment on the sass.
} | ||
} | ||
|
||
@keyframes pulse { |
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.
Nit. Might want to prefix these similar to the rest of the sass?
💚 Build SucceededMetrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: |
FYI I changed the "Exit feature editing". Now we're showing an "Edit features" label (same from the popover) and then an "Exit" after it. This solution was suggested by @mdefazio and I think it's more consistent with what we're showing inside the popover. |
) * Timeslider icons and styles * Exit feature editing. Animations for toolbar and timeslider * Removing unnecessary line * Adding padding to tooltip field popover * Adding pulse animation on open timeslider * Adding isDraggable to timeslider * More timeslider styles * Better positioning of the exit edit mode button * Enable edit mode when new vector layer added * Review feedback. Update action name. Remove unneeded component state * Minor updates. One more action for cancel. Type updates. Snapshot update * fixing tests and eslint error * Added new exit mode design. Renamed animations * Features instead of feature to be consistent with popover * fix type error Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Aaron Caldwell <aaron.caldwell@elastic.co>
…103716) * Timeslider icons and styles * Exit feature editing. Animations for toolbar and timeslider * Removing unnecessary line * Adding padding to tooltip field popover * Adding pulse animation on open timeslider * Adding isDraggable to timeslider * More timeslider styles * Better positioning of the exit edit mode button * Enable edit mode when new vector layer added * Review feedback. Update action name. Remove unneeded component state * Minor updates. One more action for cancel. Type updates. Snapshot update * fixing tests and eslint error * Added new exit mode design. Renamed animations * Features instead of feature to be consistent with popover * fix type error Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Aaron Caldwell <aaron.caldwell@elastic.co> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Aaron Caldwell <aaron.caldwell@elastic.co>
Summary
This PR closes #102537.
Draw shape mode
Timeslider
Preview
Screen+Recording+2021-06-28+at+07.17+PM.mov
Missing
Checklist