Conversation
Passing run #2312 ↗︎
Details:
Review all test suite changes for PR #846 ↗︎ |
|||||||||||||||
7939943 to
c3865c5
Compare
|
Should be a bit shrinked |
|
@ltshb Done. I was also thinking: the dropdown probably isn't very user friendly when it contains 150+ entries. Maybe we can (some time) search for other solutions. Some spontaneous ideas:
(speaking of entering numbers: should we use |
9820f8c to
483f343
Compare
ltshb
left a comment
There was a problem hiding this comment.
It kind of work on the tablet, but the buttons are a bit small.
One general issue is that when typing a number in the dropdown and press enter it action the play button instead of validating the entry. This happens when you enter an invalid number like 100. In this case the field should be marked as invalid as on the slider.
0a62c4b to
5601e18
Compare
ltshb
left a comment
There was a problem hiding this comment.
Very nice, just two minor issues:
- Button class consistency
- When entering an invalid year input, the tippy for invalid year is shown (which is correct), but then if you change the language the tippy lang is updated and stays display but the year is reseted to the previous selected year which is valid. In this case we need either to not reset to the previous selected year or also reset the tippy
| }) | ||
| } | ||
| }) | ||
| tippyTimeSliderInfo?.destroy() |
There was a problem hiding this comment.
I find dangerous to put the tippy destroy in a function which could be called anytime in the future. I would let the tippy destroy in the onMounnted callback it is safer for the future.
There was a problem hiding this comment.
Oh yes, that's a rebasing error, thanks!
|
|
||
| <div class="input-group-append btn-group btn-group"> | ||
| <button | ||
| class="btn border btn-light d-flex align-items-center rounded-start-0 rounded-end-0" |
There was a problem hiding this comment.
For code consistency you should use btn-outline-group instead of btn-light for button groups.
There was a problem hiding this comment.
OK! The name is a bit misleading ;)
871dce0 to
22c26eb
Compare
The play button icon didn't reset to "play" after pausing the playing of the years
The data flow of the time slider was overly complicated and sometimes lead to triggering setting the same value twice. Tried to remodel the route that the data takes. Everything now more or less writes to the same computed property, which triggers setting the store if the data is correct. In my opinion, with this solution, there are less side effects and (almost) a single source of truth for the year. Two watchers could be eliminated.
The slider allows for selecting more years that are potentially available on all the displayed layers. When unmounting the slider, we shouldn't be setting the layers to invalid years.
Replace the simple time slider select with a searchable dropdown similar to the one in the provider urls
If the year cursor is moved, the store gets updated after a slight amount of time. But it isn't necessary to release the mouse anymore, the map will update when only the cursor is moved
Debounce the input year validation a bit so that the error isn't immediately shown when the user starts typing
22c26eb to
c4e76f4
Compare
On small screens (but not mobile) show a Dropdown instead of the time slider.
Test link