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

[#5] Logo Grid UI #28

Merged
merged 6 commits into from
Apr 25, 2024
Merged

[#5] Logo Grid UI #28

merged 6 commits into from
Apr 25, 2024

Conversation

empee3
Copy link
Contributor

@empee3 empee3 commented Apr 24, 2024

Summary

This PR pulls in Blueprint styles for the Logo Grid Component.

Issues

Testing Instructions

  1. Create a new page or edit an existing page
  2. Insert the Logo Grid block
  3. Upload photos to the image blocks inside of the component
  4. Verify that:
    • Images resize correctly in the grid
    • Columns are responsive (two columns until large breakpoint, then four columns to match Blueprint)
    • Justification can be changed on the component and images will be positioned accordingly

Screenshots

Two columns:

Screenshot 2024-04-24 at 3 43 44 PM

Four columns:

Screenshot 2024-04-24 at 3 43 29 PM

With Aspect Ratio:

Screenshot 2024-04-25 at 10 50 55 AM

@empee3 empee3 self-assigned this Apr 24, 2024
@layer components {
.acf-block-logo-grid figure {
@apply flex;
@apply basis-1/2-gap lg:basis-1/4-gap;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we can only use flexbox layout with components, these classes have been added to simulate grid-cols- from Blueprint.

Comment on lines +43 to +49
flexBasis: {
'1/2-gap': 'calc((100%/2) - var(--wp--style--block-gap))',
'1/4-gap': 'calc((100%/4) - var(--wp--style--block-gap))',
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Column widths cannot be calculated correctly unless the gap is subtracted from the column width.

@empee3
Copy link
Contributor Author

empee3 commented Apr 24, 2024

I may be overthinking, but should we also add an aspect ratio to the images to keep them a consistent height? This could be assumed from Blueprint, but the placeholder images are all of the same height and width.

Comment on lines 12 to 19
/* Fix for image sizing in editor */
.acf-block-logo-grid .components-resizable-box__container {
@apply !shrink;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Images remain at full width in the editor because of the .components-resizable-box__container classes [see discussion].

Copy link
Contributor

Choose a reason for hiding this comment

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

We have CSS nesting added so let's nest the CSS so it is less tags.

@nathan-schmidt-viget
Copy link
Contributor

@empee3 Agreed it looks a bit weird without an aspect ratio. If we can set them in TW so the img is set to object-fit: contain; the parent element as the aspect ratio that would be awesome.

@empee3 empee3 marked this pull request as ready for review April 25, 2024 14:45
Copy link
Contributor

@nathan-schmidt-viget nathan-schmidt-viget left a comment

Choose a reason for hiding this comment

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

LGTM!

@empee3 empee3 merged commit 692e0e9 into main Apr 25, 2024
@empee3 empee3 deleted the mp/5-logo-grid-ui branch April 25, 2024 17:32
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.

2 participants