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

updated margin below DonateLandingPage featured image #11707

Merged
merged 3 commits into from
Jan 11, 2024

Conversation

danielfmiranda
Copy link
Collaborator

@danielfmiranda danielfmiranda commented Jan 10, 2024

Description

Link to sample test page: https://foundation-s-feature-10-lyr5pm.herokuapp.com/en/donate/
Related PRs/issues: #10364

This PR updates the margin below the Donate Landing Page's "featured image" to tw-mb-12.

Per request of design, this update:

  • Renders a space of 32px between the featured image and page title on mobile
  • Renders a space of 24px between the featured image and the page intro on tablet
  • Renders a space of 16px between the featured image and the page intro on tablet

Screenshots:

Before:

Mobile Spacing (Note the gap of ~24px):
Screenshot 2024-01-09 at 4 52 37 PM

Desktop Spacing (Note the gap of ~16px):
Screenshot 2024-01-09 at 4 57 20 PM

After:

Mobile Spacing (Note the updated gap of ~32px):

Screenshot 2024-01-10 at 5 15 19 PM

Tablet Spacing (Note the updated gap of ~24px):
Screenshot 2024-01-10 at 5 15 03 PM

Desktop Spacing (Note the updated gap of ~16px):

Screenshot 2024-01-10 at 5 14 45 PM

@mmmavis
Copy link
Collaborator

mmmavis commented Jan 10, 2024

Hi @danielfmiranda , from the code and the rendered page, it seems like on both mobile and desktop, the margin is 24px. Maybe I'm looking at the wrong place?

image

Copy link
Collaborator

@nancyt1 nancyt1 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! Thanks!

@danielfmiranda danielfmiranda temporarily deployed to foundation-s-feature-10-lyr5pm January 11, 2024 01:07 Inactive
@danielfmiranda
Copy link
Collaborator Author

danielfmiranda commented Jan 11, 2024

Thanks for the catch @mmmavis! After taking a look, I found that I have actually gotten the spacing confused. According to the mockups that Nancy linked in #10364, the spacing should be:

  • 16px desktop
  • 24px tablet
  • 32px mobile

I have updated the image with the following classes tw-mb-16 medium:tw-mb-12 large:tw-mb-8 (screenshots have been updated in PR description) and have tagged @nancyt1 for a quick re-review!

Copy link
Collaborator

@nancyt1 nancyt1 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! Thanks Daniel!

Copy link
Collaborator

@mmmavis mmmavis left a comment

Choose a reason for hiding this comment

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

🎉

@danielfmiranda danielfmiranda temporarily deployed to foundation-s-feature-10-lyr5pm January 11, 2024 18:27 Inactive
@danielfmiranda danielfmiranda merged commit d280662 into main Jan 11, 2024
6 checks passed
@danielfmiranda danielfmiranda deleted the feature/10364-updated-donate-title-spacing branch January 11, 2024 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants