Skip to content
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

[Content] Spring 2024 tornado season data story #420

Merged
merged 200 commits into from
Dec 6, 2024
Merged

Conversation

acblackford
Copy link
Contributor

Why are you creating this Pull Request?

Copy link

netlify bot commented Jul 17, 2024

Deploy Preview for visex ready!

Name Link
🔨 Latest commit 928071e
🔍 Latest deploy log https://app.netlify.com/sites/visex/deploys/67522b074ab4f90008f68833
😎 Deploy Preview https://deploy-preview-420--visex.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@aboydnw
Copy link
Contributor

aboydnw commented Dec 5, 2024

@acblackford I'm going through and testing now, since it's short notice I'll post this comment now before I'm done testing. First thing I notice is that all the datasets could use an infoDescription

See the recent PR for NLDAS for an example. #498 My suggestion would be to use the bulleted list of "Dataset Details" that you have in the body of each dataset mdx file already.

@aboydnw
Copy link
Contributor

aboydnw commented Dec 5, 2024

@acblackford the only other thing that I notice is that the email link at the bottom probably is not functioning as intended. It just opens a page with mailto appended at the end of the URL. I actually see recent stories with it, like Black Belt, also behaving this way.

Off the top of my head I'm not sure how to solution this, or if we have encountered this in the past. Do you have any ideas? Maybe for a short term solution we just remove it as a link?

@acblackford
Copy link
Contributor Author

@aboydnw All datasets should now have info descriptions added

Copy link
Contributor

@aboydnw aboydnw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Also a really interesting story and data. Nice work. Let's include in the release next week @snmln

(based on @anayeaye telling me this is ready to go)

@acblackford acblackford merged commit d0c18fe into develop Dec 6, 2024
6 checks passed
@acblackford acblackford deleted the tors2024 branch December 6, 2024 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants