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

Share your story refresh #3836

Merged
merged 5 commits into from
Nov 29, 2024
Merged

Conversation

kccrtv
Copy link
Collaborator

@kccrtv kccrtv commented Nov 26, 2024

Description and Motivation

continues with news page refresh. condenses the submission guidelines and adds numbering for easy reference. changed the button to secondary, and included buttons both at the top and bottom of the page.

Has this been tested? How?

tests passing

Screenshots (if appropriate)

mobile
desktop

Types of changes

  • New content or feature
  • Refactor / chore

New frontend preview link is below in the Netlify comment 😎

Copy link

netlify bot commented Nov 26, 2024

Deploy Preview for health-equity-tracker ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 6450829
🔍 Latest deploy log https://app.netlify.com/sites/health-equity-tracker/deploys/674a503443568e0008b7a49e
😎 Deploy Preview https://deploy-preview-3836--health-equity-tracker.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.

@kccrtv kccrtv changed the title Share your story Share your story refresh Nov 26, 2024
@kccrtv kccrtv marked this pull request as ready for review November 26, 2024 22:32
Copy link
Collaborator

@benhammondmusic benhammondmusic left a comment

Choose a reason for hiding this comment

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

visually looks awesome. make sure to switch the nesting og the ul > div li as uls should only contain ol or li children. also i think eventually we should refactor the button/link/mail button situation because all the condition logic is really complex and prone to failure (as we've seen lol). i also added an issue to update the loading skeleton stuff to match the new layout as it's off now #3837 but that can be done later and shouldn't hold up this PR

frontend/src/pages/News/ShareYourStory.tsx Outdated Show resolved Hide resolved
frontend/src/styles/HetComponents/HetButtonSecondary.tsx Outdated Show resolved Hide resolved
@kccrtv kccrtv merged commit 7965791 into SatcherInstitute:main Nov 29, 2024
8 checks passed
@kccrtv kccrtv deleted the share-your-story branch November 29, 2024 23:39
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.

2 participants