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.

Sign up for GitHub

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

Jump to bottom

Fix account header spacing when badges present #5954

Merged
merged 3 commits into from
Feb 17, 2022
Merged

Fix account header spacing when badges present #5954

merged 3 commits into from
Feb 17, 2022

Conversation

aduth
Copy link
Contributor

@aduth aduth commented Feb 15, 2022

Partly a regression of #5900

This fixes a handful of issues related to how the account header is shown:

  • Removes empty space for identity-verified accounts, since previously showing_any_partials? would account for show_pii_partial? despite the fact this is not rendered in the account header
  • Restores extra spacing on mobile for "Welcome" message when badge is visible, regressed in LG-4795: Change font to Public Sans #5900
    • This time, however, the extra spacing is only added when badges are visible. Previously, it was always shown.
  • Reduces margin between account header and the subsequent page heading, from 4rem to 2rem.

Screenshots:

Desktop:

State Before After
Verified localhost_3000_account (2) localhost_3000_account (3)
With message localhost_3000_account (1) localhost_3000_account

Mobile:

State Before After
Verified localhost_3000_account(iPhone XR) (2) localhost_3000_account(iPhone XR) (3)
With message localhost_3000_account(iPhone XR) localhost_3000_account(iPhone XR) (1)

anniehirshman-gsa
anniehirshman-gsa approved these changes Feb 15, 2022
View reviewed changes
Copy link
Contributor

@anniehirshman-gsa anniehirshman-gsa left a comment

Choose a reason for hiding this comment

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

Looks great to me!

@aduth
Add changelog
d496d6f 
changelog: Bug Fixes, Account, Fix account header layout when badges are visible
zachmargolis
zachmargolis requested changes Feb 16, 2022
View reviewed changes
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

app/presenters/account_show_presenter.rb Outdated Show resolved Hide resolved
zachmargolis
zachmargolis approved these changes Feb 16, 2022
View reviewed changes
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM (whoops wrong button)

@aduth aduth merged commit 29c200d into main Feb 17, 2022
@aduth aduth deleted the aduth-account-header branch February 17, 2022 13:16
@aduth aduth mentioned this pull request Feb 18, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zachmargolis zachmargolis zachmargolis approved these changes

@anniehirshman-gsa anniehirshman-gsa anniehirshman-gsa approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aduth @zachmargolis @anniehirshman-gsa