-
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
EIC Features > main #752
Merged
Merged
EIC Features > main #752
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
Co-authored-by: Daniel da Silva <[email protected]>
Add Content Override for Stories Hub page for EIC. I also got rid of page local nav. Close NASA-IMPACT/veda-config-eic#6
It seems EIC will require plenty of Embedded content. This PR wraps `Embed` component (which is essentially iframe) into `<Lazyload >` element, so embedded contents can load only when it is in the view. Lazy embed example: https://deploy-preview-743--veda-ui.netlify.app/stories/life-of-water
Rather than dealing with 'multi catalog' issue, this PR narrows down the scope and makes the layer to be responsible to bring stac and tile endpoints when needed. (`stacApiEndpoint` and `tileApiEndpoint` to data mdx.) ~~We are using `stacCol` as an identifier in layer fetching logic, especially in `context/layer-data`. This will make a problem if there exist the same collection ids in different stac instances. So, we need to figure out a unique identifier for the layer fetching. But with the new A&E page coming and everything, I feel more biased towards documenting this as a caveat (no duplicate stac collection IDs are allowed) and moving forward for now. What do you think @danielfdsilva ? @sandrahoang686 ?~~ : Welp 💦 It was less amount of effort than I thought so I just went ahead and added 'stacEndpoint' as an additional identifier. ex. Previous attempt to implement multi-catalog (from @slesaad ) : https://github.com/NASA-IMPACT/veda-config/pull/301/files
I realized that there are multiple props that iframe can accept to make the communication safe/ more compatible with standard (such as https://github.com/NASA-IMPACT/veda-config-eic/pull/23/files#diff-9a900b0095d3eecf847dc69b3300d909fb529d47ce3ffe375330081d0bcdee76R145) This PR makes all those props be passed to iframe element.
I made this change because of the strong request from EIC. I am very hesitant about suggesting this change because there is no way to prevent the navigation bar from breaking with this change when a user passes too many 'navMainItem' (or even items with long names). We have to clearly communicate that veda-ui is not in charge of the breaks that can be introduced from`navMainItem`. Look at how it can look like : NASA-IMPACT/veda-config-eic#24
✅ Deploy Preview for veda-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
danielfdsilva
requested changes
Nov 22, 2023
app/scripts/components/analysis/define/use-stac-collection-search.ts
Outdated
Show resolved
Hide resolved
app/scripts/components/analysis/define/use-stac-collection-search.ts
Outdated
Show resolved
Hide resolved
danielfdsilva
approved these changes
Nov 28, 2023
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.
@hanbyul-here Loos good! let's solve the conflicts and get this merged! 🦾
Follow up of #749 Handles this issue: #749 (comment)
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.
Background for this pr: As new A&E launch being delayed, it will be better for all the changes to stay on the same branch, and release the version properly so we don't have to manage multiple feature branches at the same time.
The change includes