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

Update Delphi website carousel #924

Merged
merged 2 commits into from
Feb 15, 2024
Merged

Update Delphi website carousel #924

merged 2 commits into from
Feb 15, 2024

Conversation

rzats
Copy link
Contributor

@rzats rzats commented Feb 5, 2024

Implements the changes to the website carousel requested in https://cmu-delphi.atlassian.net/browse/OKRS24-8

Copy link

netlify bot commented Feb 5, 2024

Deploy Preview for cmu-delphi-main ready!

Name Link
🔨 Latest commit b5d45f7
🔍 Latest deploy log https://app.netlify.com/sites/cmu-delphi-main/deploys/65ce3a5b23aa2b00089aa1ba
😎 Deploy Preview https://deploy-preview-924--cmu-delphi-main.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.

Copy link
Contributor

@carlynvandyke carlynvandyke 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! Will leave for Ron and George to take a look.

@rlunde
Copy link
Contributor

rlunde commented Feb 5, 2024

Thanks - that looks good. We may want to make more changes to names (pending feedback from Roni and others) but this looks like a good first step!

Copy link
Contributor

Choose a reason for hiding this comment

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

I think a more meaningful file name that's similar to the other file names in that directory might be: "landing-pg-hero-introducing_epidata.jpg"

Copy link
Contributor

Choose a reason for hiding this comment

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

I think a more meaningful file name that's similar to the other file names in that directory might be: "landing-pg-hero-exploratory_analysis.jpg"

Copy link
Contributor

@rlunde rlunde left a comment

Choose a reason for hiding this comment

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

I thought about it a bit more, and while the image files in that folder aren't very consistent in naming format, we could choose file names that match the text that people see (see comments on the image files).

@rlunde rlunde self-requested a review February 15, 2024 16:27
@rlunde
Copy link
Contributor

rlunde commented Feb 15, 2024

Looks good - thanks!

@rzats rzats merged commit 88ce1ff into dev Feb 15, 2024
7 checks passed
@rzats rzats deleted the rzatserkovnyi/carousel-update branch February 15, 2024 16:30
content/_index.md Show resolved Hide resolved
Comment on lines +28 to +30
title: An Open Repository of Real-Time COVID-19 Indicators
link: https://delphi.cmu.edu/blog/2022/12/14/introducing-epidata-v4/
alt: Introducing Epidata
Copy link
Contributor

Choose a reason for hiding this comment

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

This title isnt appropriate because the article is not at all COVID specific... And then do we want to move this one slot closer to the beginning of the carousel (in front of the COVID-titled article)?

Suggested change
title: An Open Repository of Real-Time COVID-19 Indicators
link: https://delphi.cmu.edu/blog/2022/12/14/introducing-epidata-v4/
alt: Introducing Epidata
title: Introducing Epidata
link: https://delphi.cmu.edu/blog/2022/12/14/introducing-epidata-v4/
alt: View Post

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch! I do think that moving it closer to the beginning is a good idea, too.

This was referenced Feb 20, 2024
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