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

refactor: card image demo grid #296

Merged
merged 7 commits into from
Jan 10, 2024
Merged

Conversation

wesleyboar
Copy link
Member

@wesleyboar wesleyboar commented Jan 10, 2024

Overview

  • Simplify grid in demo of c-card--images
  • Avoid shrink / overflow of "contact card" images.

Related

Changes

  • changed card images demo to use s-image-grid not o-grid
  • simplified two-column grid tweak
  • fixed contact card tweak having no effect
  • added support for s-grid

Testing

  1. Open http://localhost:4000/components/preview/c-card--images.
  2. Test window at different widths (resize or zoom).
  3. Verify grid always has two columns.
  4. Verify contact card image is never shorter nor wider than its card.
  5. Check that nothing else broke.

UI

Narrow Medium Wide
narrow medium wide

@github-actions github-actions bot added the refactor Re-writes/structures code but retains behavior label Jan 10, 2024
@wesleyboar wesleyboar added the patch A backward-compatible fix label Jan 10, 2024
@wesleyboar wesleyboar marked this pull request as ready for review January 10, 2024 16:57
@wesleyboar wesleyboar changed the title Refactor/contact card demo grid refactor: card image demo grid Jan 10, 2024
Comment on lines 33 to 31
.card--image-top {
[class*="card--image-top"] {
Copy link
Member Author

Choose a reason for hiding this comment

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

The .card--image-top had no effect, because demo uses longhand classes (.c-card .c-card--…) not shorthand classes.

Comment on lines -14 to 16
#image-cards {
grid-template-columns: 1fr 1fr;
.s-image-grid {
--max-cols: 2;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I think s-image-grid achieves the goal with less demo code.

Copy link
Member Author

Choose a reason for hiding this comment

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

An id must be unique in the whole document. I replaced it with an existing class.

Copy link
Collaborator

@R-Tomas-Gonzalez R-Tomas-Gonzalez 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.

@wesleyboar wesleyboar merged commit 9b44e31 into main Jan 10, 2024
@wesleyboar wesleyboar deleted the refactor/contact-card-demo-grid branch January 10, 2024 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch A backward-compatible fix refactor Re-writes/structures code but retains behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants