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

USWDS - Card: Fix header first media overlap #5423

Merged
merged 4 commits into from
Sep 28, 2023

Conversation

mahoneycm
Copy link
Contributor

@mahoneycm mahoneycm commented Aug 3, 2023

Summary

Fixed a margin bug on header-first variants that caused exdent card media to overlap headers.

Breaking change

This is not a breaking change.

Related issue

Closes #5401

Related pull requests

Changelog →

Note*
Bug discovered while working on #5325 but has no overlap; They can be merged separately without error.

Preview link

Preview link:
Demo repo →
Card component →

Problem statement

When header first variants for card components have large borders, the media overlaps the header. This is caused due to exdent styles setting a negative top margin to counteract the border. Because header-first variants will always have the media after the header, we should remove the negative top margin for this variant.

Solution

Specify 0 top margin on exdent media within header-first variants.

Testing and review

  1. View demo repo preview →
  2. Confirm media does not overlap header
  3. Inspect usa-card__media--exdent class
  4. Remove top-margin: 0.
  5. Confirm media shifts to overlap header
  6. (optional) - Checkout test repo and update $theme-card-border-width in settings to various values and confirm header is always visible

Testing checklist

  • header-first card variants media never overlaps header
  • Logic behind solution is valid: Media on header-first cards should never have negative margin

Screenshots

Screenshots

Before

image

After

image

Copy link
Contributor

@amyleadem amyleadem left a comment

Choose a reason for hiding this comment

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

This makes sense to me. Thanks for the fix!

Copy link
Contributor

@mejiaj mejiaj 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. Tested header first with media exdent on both regular and flag variants.

Copy link
Member

@thisisdano thisisdano left a comment

Choose a reason for hiding this comment

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

Nice!

@thisisdano thisisdano merged commit 69b76d5 into develop Sep 28, 2023
3 checks passed
@thisisdano thisisdano deleted the cm-media-header-cutoff branch September 28, 2023 22:04
@thisisdano thisisdano mentioned this pull request Sep 29, 2023
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.

USWDS - Bug: Cards with large borders cut off headers
4 participants