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

<rh-card>: incorrect margins / breakpoints #2007

Open
zeroedin opened this issue Oct 23, 2024 · 3 comments
Open

<rh-card>: incorrect margins / breakpoints #2007

zeroedin opened this issue Oct 23, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@zeroedin
Copy link
Collaborator

zeroedin commented Oct 23, 2024

@max-messmer reported in the update on RHDC from previous version of RHDS to 2.1.0 that rh-card appeared to have a smaller margin.

I believe the problem lies in at point we switched from @media queries to @container queries but didn't take into account that the swap wasn't as simple as a 1:1. The spacing guide (although itself having problems) shows a desktop size and a mobile size, the former with a margin-inline: 32px and later margin-inline: 24px. This break originally happened at a media query of 768px. However given a direct swap to @container this break point wouldn't be hit unless the card itself was that wide. Given the documentation my assumption is the correct fix for this would be to reduce the container query to:

@container card (min-width: 320px) {
  #header,
  #body,
  #footer {
    margin-inline: var(--rh-space-2xl, 32px);
  }

  #header,
  #body {
    margin-block-start: var(--rh-space-2xl, 32px);
  }

  #footer {
    margin-block-end: var(--rh-space-2xl, 32px);
  }
}

If the card then is less than 320px wide the margin-inline would reduce.

I created this codepen to demonstrate the issue

Thanks @max-messmer for reporting this issue.

@zeroedin zeroedin added the bug Something isn't working label Oct 23, 2024
@zeroedin zeroedin added this to the 2024/Q4 — Cubone release milestone Oct 23, 2024
@bennypowers
Copy link
Member

We should confirm with @coreyvickery or @marionnegp what the correct behaviour should be, emphasizing the switch from "mobile vs desktop" to container size

@coreyvickery
Copy link
Collaborator

@zeroedin Easy to do 32px of padding on desktop (≥768px) and 24px of padding on mobile (≤768px)?

cc @bennypowers @marionnegp

@marionnegp
Copy link
Collaborator

@bennypowers @zeroedin, is there a reason it's better to do container queries than media queries for card? Switching to container queries kind of messes with stuff like tile too, which should have similar spacing to card.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Backlog
Development

No branches or pull requests

4 participants