Add Logo Group Object #1415
Add Logo Group Object #1415
Conversation
🦋 Changeset detectedLatest commit: 71b03a6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Co-authored-by: Gerardo Rodriguez <[email protected]>
Co-authored-by: Gerardo Rodriguez <[email protected]>
| display: flex; | ||
| flex-wrap: wrap; | ||
| justify-content: center; | ||
| margin: (-1 * ms.step(3)); |
There was a problem hiding this comment.
This addresses my earlier feedback, but I notice now that this component needs a lot of padding to avoid horizontal scrollbars. Maybe it makes sense to add the padding back to the parent element (instead of requiring utilities), we should just make sure to include adequate comments explaining why... for example, "we add generous padding to balance out the whitespace differentiating brands" or something to that effect.
There was a problem hiding this comment.
I removed the negative margin, curious what you think of it now?
There was a problem hiding this comment.
@dromo77 Did you push that change? I still see the negative margin.
There was a problem hiding this comment.
No 😆 should be there now.
| /** | ||
| * Because there is generous spacing between the logos, we added a lot of white | ||
| * space around Logo Group to help balance the pattern visually. Including the | ||
| * spacing as part of the pattern means we don't need to rely on utilities. | ||
| */ |
There was a problem hiding this comment.
This comment feels a little odd since there isn't any padding around the logo group. Maybe it should be rewritten and put closer to the items themselves (which have margin)?
| } | ||
|
|
||
| .o-logo-group > * { | ||
| margin: ms.step(3); |
There was a problem hiding this comment.
Should this be a design token? Maybe size.spacing.gap.logo_group?
Overview
This PR adds a new object called Logo Group for displaying a group of logos together.
Screenshots
Testing