-
Notifications
You must be signed in to change notification settings - Fork 1
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
Connect MDX components, add additional stories & datasets, update homepage, and add assets #34
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for veda-ui-next-test ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@aboydnw closed the original PR in favor of this one and left a comment on the closed PR on why. But re-iterating here that I did this because to debug the various bugs, I had to add content 1 by 1 and fix each issue as I went so this was easiest for me. If you can confirm that i've added all the content you had originally in the old PR and validate the preview, thanks! Note: ScrollytellingBlock in airquality story styling needs to be fixed but I've added that as part of NASA-IMPACT/veda-ui#1282 to get this out and not add more on top but lmk if you'd like otherwise. |
export function EnhancedScrollyTellingBlock(props) { | ||
const { datasets } = useDataStore(); | ||
const transformed = {}; | ||
datasets?.map((dataset) => { |
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.
👋 Considering Mapblock and Scrollytelling expects the same type of the dataset as a prop, (My PR here might be helpful: NASA-IMPACT/veda-ui#1361) but we have to pass different shape of the data to each block, I think something is wrong. Would you be able to investigate what is going on? (Or am I missing anything?)
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.
I reviewed your PR and left a comment but made some changes to get it merged in. We are missing something because the same data structure breaks which is why I had to do these unfavorable different transformations but I could investigate further. I didn't want to change much in veda-ui related to this but if its small enough, could be done.
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.
I added a few tweaks to this ticket so we don't lose track of them, but I think we can address them later
Only thing I am seeing with this PR is that only the first frame of the scrollytelling block is loading for me in the air quality story. The content and frames seem to all be there, but the only frame that loads anything in the map (basemap or layer) is the first one.
@aboydnw i addressed that here but if thats a blocker and want it shipped with this PR, lmk 👍🏼 |
Connected with Sandra in slack and she let me know the issue I spotted is what was meant by scrollytelling styling in her original comment, and will be addressed with NASA-IMPACT/veda-ui#1282 I adjusted the language there to be more specific and we can address that later |
Available PR templates
Related Issues:
Related PR: NASA-IMPACT/veda-ui#1358
This work is an extension off of @aboydnw original PR that adds content.
Changes:
Related PRs:
#34