Lazyload SmartLink to avoid loading react-router-dom + use a proper linkProperties #1160
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. |
dzole0311
approved these changes
Sep 19, 2024
| onLinkClick, | ||
| pathAttributeKeyName: 'to', | ||
| linkComponent: SmartLink | ||
| // @ts-expect-error SmartLink needs to be lazily loaded temporarily to prevent ui-library from loading react-router-dom | ||
| LinkElement: SmartLink |
There was a problem hiding this comment.
Relooked at your previous PR - so looks like we have to default to SmartLink here because GHG is unable to pass this element into the card? We may just want to update this on GHG side to pass the LinkElement in instead of this non-ideal temporary solution? Unsure if thats too much of a lift right now though so this comment is non-blocking. But could we leave a @TODO or @NOTE too here with the same note you have?
There was a problem hiding this comment.
Yeah, it will be best to just change the code from the instance side. But I think it is easy to miss it with R3 and everything, so put the temporary measure here. I left a note.
sandrahoang686
approved these changes
Sep 20, 2024
hanbyul-here
force-pushed
the
fix/card
branch
from
c6ef2e4
to
ce1b13b
Compare
export SmartLink from instance
hanbyul-here
force-pushed
the
fix/card
branch
from
ce1b13b
to
e7065ff
Compare
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: follow up of follow up -_- #1159
Description of Changes
Urg I used wrong attributes, so the previous fix didn't really fix the problem.
The previous approach broke the library build too by including SmartLink, which uses react-router-dom. I think I worked around by lazy loading the component in this PR. I tested it locally but if you can, please give it another test 🙇
Notes & Questions About Changes
Validation / Testing
ok, this time, I actually created a preview on GHG; this is the page that uses Card component: https://deploy-preview-596--ghg-demo.netlify.app/learn