Expose Data Catalog and update child components to pass in routing/nav #1096
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for veda-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
sandrahoang686
changed the title
sandrahoang686
added a commit
that referenced
this pull request
Aug 30, 2024
**Related Ticket:** Very vaguely related to #946 This PR makes the Card component delegate what link component to render to the `SmartLink` component. I initially started working on this PR to continue the work on #1096, more specifically, to make the link component come from props. I thought this change was worth merging regardless of whether we work on the routing problem now or later.
sandrahoang686
force-pushed
the
refactor/1085-expose-data-catalog
branch
from
6ce5f21 to
5b3fc8d
Compare
hanbyul-here
reviewed
Sep 4, 2024
| let navCommit = navigate; | ||
|
|
||
| if (push) { | ||
| navCommit = ({ search }) => navigate.push(`?${search}`); |
There was a problem hiding this comment.
I think there is a potential to make it even more flexible if we just let the instance to pass this whole thing as navigate props. ex. Replace this line: https://github.com/developmentseed/next-veda-ui/pull/3/files#diff-6578b281e50705562cdf8ba73c73ae2bb1df366e18e91e9595106b09cfbf3f89R14
const controlVars = useFiltersWithQS({navigate: ({ search }) => router.push(`?${search}`)});
then let's say if the instance wants to replace the url instead of push, it can handle it on its level. (and no additional push props is needed) What do you think?
There was a problem hiding this comment.
I think that also makes sense but I think we should work on closing out the PR if this is not blocking we can address this in the routing ticket / make note of this. Is that fine with you?
hanbyul-here
reviewed
Sep 5, 2024
hanbyul-here
reviewed
Sep 5, 2024
hanbyul-here
approved these changes
Sep 5, 2024
dzole0311
requested changes
Sep 5, 2024
There was a problem hiding this comment.
I think I spotted a bug which I can't see in the rest of the deployments:
- Go to Data Catalog
- Click on "CASA-GFED3 Land Carbon Flux"
- Click on "Explore Data"
- I get an error page with
Cannot destructure property 'pathname' of 'e' as it is undefined
The button should lead to the E&A page with the dataset filtered.
|
@dzole0311 thanks for catching, fixed ✅ |
|
Looks good to me! I clicked around the data catalog/overview, stories, and E&A and didn't. notice anything off |
hanbyul-here
added a commit
that referenced
this pull request
Sep 18, 2024
**Related Ticket:** Follow up of #1096 & d84527b#diff-7d1c949cac222e52ef45864275aff43e6fa94599bd4684d9611fa25b610e900b ### Description of Changes - I accidentally removed the question mark operator while running a lint [in the previous pr]( d84527b#diff-7d1c949cac222e52ef45864275aff43e6fa94599bd4684d9611fa25b610e900b) and this operator is actually needed. I put the proper type so it doesn't happen with automatic lint - I made the Card Component work backward compatible since the component is exposed to instance levels. - Simplified parameters for `useFiltersWithQS` ### Notes - I found another bug on filters that the selected value is not being displayed, but it can be handled separately. - I checked the library build locally, but didn't publish it yet. will do after the review.
sandrahoang686
added a commit
that referenced
this pull request
Sep 25, 2024
## 🎉 Features - [E&A] Implement colormap configurability #1117 - [E&A] Feature flag colormap configurability #1147 - Add font md size as 20px for new Design System #1125 ## 🚀 Improvements - [E&A] Return default value of colormap along with other settings #1128 - Create new raster paint layer module and factor out BaseTimeseriesProps #1105 - Consolidate a place where to check linkProps #1121 #1160 - Expose Data Catalog and update child components to pass in routing/nav for library build #1096 #1159 - Set up eslint rule for no trailing spaces #1146 - Replace cmr-stac with titiler-cmr #1131 ## 🐛 Fixes - Update external link in top navigation target #1145 - Update PageHeader/Nav to not throw #1149 - [E&A] Fix to convert the time to usertzdate #1151 - Use sourceparams as it is when it is there #1148 - Fix compare label on block map #1153 - Discard the previous sourceExclusive value to fix the case when the datasets with different sourceExclusive can be selected together #1161 - Show the name of the selected filter on story hub #1161
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related Ticket: #1085
Description of Changes
Notes & Questions About Changes
Validation / Testing