-
Notifications
You must be signed in to change notification settings - Fork 6
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
Timeline date selection centering functionality #1181
Conversation
✅ Deploy Preview for veda-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Nice work @snmln! I did a quick test and everything works as expected 👍 I've left a few comments with feedback below and will run one more test before merging, just to double-check.
app/scripts/components/exploration/components/timeline/timeline-controls.tsx
Outdated
Show resolved
Hide resolved
app/scripts/components/exploration/components/timeline/timeline-controls.tsx
Outdated
Show resolved
Hide resolved
app/scripts/components/exploration/components/timeline/timeline-controls.tsx
Outdated
Show resolved
Hide resolved
app/scripts/components/exploration/components/timeline/timeline-controls.tsx
Outdated
Show resolved
Hide resolved
app/scripts/components/exploration/components/timeline/timeline-controls.tsx
Outdated
Show resolved
Hide resolved
app/scripts/components/exploration/components/timeline/timeline-controls.tsx
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.
Small nit left, otherwise looks good from my side 👍
Heads up that I had to merge main
into this branch locally to test the feature using the latest fixes we have without seeing errors in some cases.
app/scripts/components/exploration/components/timeline/timeline-controls.tsx
Outdated
Show resolved
Hide resolved
You can find VEDA Dashboard preview link to test these changes: NASA-IMPACT/veda-config#470 ## 🎉 Features - Cookie consent component: #1163, #1165, #1127, #1177 - Stories Hub export in #1162 - Expose As Is - PageHeader, LinkElement Prop & Logo in #1184 ## 🚀 Improvements - Add docs for registry dev in #1169 - Timeline date selection centering functionality in #1181 - Deprecate old explore page, Mapbox component #1176 - Update doc with new diagrams about general architecture in #1195 - Add playwright in #1139 - Pre-select AOIs that have the "selected" prop set to true in #1215 - ## 🐛 Fixes - Check dastaset analysis exclude attribute before running analysis in #1175 - Restrict dynamic colormaps to applicable layer types in #1183 - Fix external link badge display for tools hosted under the same domain in #1192 - Escape special characters in regex expression when searching datasets in #1204 ### New Contributors * @snmln made their first contribution in #1127 * @AliceR made their first contribution in #1204
Related Ticket: https://github.com/orgs/NASA-IMPACT/projects/17/views/6?pane=issue&itemId=74641280
Description of Changes
{Update with description of changes made in this pull request}
Notes & Questions About Changes
This ticket introduces
centerTimelineOnSelections
function.Validation / Testing
Navigate to exploration and select a data set. This functionality should only center when dates are selected from the date selector at the top. No centering functionality should be visible when a date in the timeline is selected via the click and drag interface.
Single date selection: Select a date that is outside of the current timeline view from the date picker. The timeline should pan to have that newly selected date now centered in the timeline.
Comparison date selection: Select a date that is outside of the current timeline view from the date picker. The timeline should pan & zoom to have that newly selected date range now centered in the timeline.
Select an 'A' date that is later in time than the existing 'B' date and then pan should still center around the 2 items.