Skip to content

PB-318: refactor time slider to composition API #742

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

Merged
merged 2 commits into from
Mar 27, 2024

Conversation

ltkum
Copy link
Contributor

@ltkum ltkum commented Mar 27, 2024

since we are working on the time Slider implementation, we might as refactor it with the composition API

Test link

@ltkum ltkum requested a review from pakb March 27, 2024 10:49
Copy link

cypress bot commented Mar 27, 2024

Passing run #1357 ↗︎

0 166 22 0 Flakiness 0

Details:

small PR corrections
Project: web-mapviewer Commit: 2aab9ef736
Status: Passed Duration: 04:14 💡
Started: Mar 27, 2024 2:05 PM Ended: Mar 27, 2024 2:09 PM

Review all test suite changes for PR #742 ↗︎

@ltkum ltkum force-pushed the compose-api-timeslider branch from 9711063 to f5867a1 Compare March 27, 2024 11:03
Comment on lines +161 to +162
// we can't watch currentYear and dispatch changes to the store here, otherwise the store gets
// dispatch too many times when the user is moving the time slider (we wait for mouseup our
// touchend to commit the change)
Copy link
Contributor

Choose a reason for hiding this comment

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

That's good enough, but now that we have some "experience" and some utils to debounce actions, you might want to plan switching to a debounce based store dispatch approach.

Relying on both mouseup and touchend can be a bit tricky on some devices that fire both

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My goal was mostly to make a 1 to 1 translation. I'll keep that in mind when I'm developing the new features, especially with the datas that are distinct.

ltkum added 2 commits March 27, 2024 14:59
since we are working on the time Slider implementation, we might as
refactor it with the composition API
@ltkum ltkum force-pushed the compose-api-timeslider branch from 426bff3 to 2aab9ef Compare March 27, 2024 13:59
@ltkum ltkum merged commit ef4ccae into develop Mar 27, 2024
6 checks passed
@ltkum ltkum deleted the compose-api-timeslider branch March 27, 2024 14:09
@cypress cypress bot mentioned this pull request Mar 27, 2024
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.

2 participants