-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add Logo Group Object #1415
Merged
Merged
Add Logo Group Object #1415
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
6811475
Add logo group object
dromo77 bffe1b3
Add changes from v-next
dromo77 6b607e1
Update changelog
dromo77 497550a
Update margin to remove extra white space around logos
dromo77 9f3a444
Update pattern structure
dromo77 b146cfc
Update logo alt text
dromo77 b18ca95
Update prototype with logo-group
dromo77 2df071a
Update size token
dromo77 2b27998
Update comments in stories
dromo77 fd427f8
Update story code
dromo77 cf7b143
Remove negative margins
dromo77 71b03a6
Update margin and comment
dromo77 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@cloudfour/patterns': minor | ||
--- | ||
|
||
Add Logo Group object. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
{% embed '@cloudfour/objects/container/container.twig' %} | ||
{% block content %} | ||
{% embed '@cloudfour/objects/logo-group/logo-group.twig' %} | ||
{% block content %} | ||
{% for item in items %} | ||
<img src="{{ item.src }}" alt="{{ item.alt }}"> | ||
{% endfor %} | ||
{% endblock %} | ||
{% endembed %} | ||
{% endblock %} | ||
{% endembed %} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
[ | ||
{ | ||
"alt": "Walmart", | ||
"src": "media/client-logos/logo-walmart.svg" | ||
}, | ||
{ | ||
"alt": "Deschutes Brewery", | ||
"src": "media/client-logos/logo-deschutes.svg" | ||
}, | ||
{ | ||
"alt": "Obama '08", | ||
"src": "media/client-logos/logo-obama.svg" | ||
}, | ||
{ | ||
"alt": "Treasure Coast Hospice", | ||
"src": "media/client-logos/logo-treasurecoast.svg" | ||
}, | ||
{ | ||
"alt": "Entertainment", | ||
"src": "media/client-logos/logo-entertainment.svg" | ||
}, | ||
{ | ||
"alt": "Hautelook", | ||
"src": "media/client-logos/logo-hautelook.svg" | ||
}, | ||
{ | ||
"alt": "Ceasefire Oregon", | ||
"src": "media/client-logos/logo-ceasefire.svg" | ||
} | ||
] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
@use "../../compiled/tokens/scss/size"; | ||
@use '../../mixins/ms'; | ||
|
||
.o-logo-group { | ||
align-items: center; | ||
display: flex; | ||
flex-wrap: wrap; | ||
justify-content: center; | ||
margin: (-1 * ms.step(3)); | ||
} | ||
|
||
.o-logo-group > * { | ||
margin: ms.step(3); | ||
dromo77 marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be a design token? Maybe |
||
width: size.$width-logo-group-item-width; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import { Story, Canvas, Meta } from '@storybook/addon-docs/blocks'; | ||
import defaultDemo from './demo/demo.twig'; | ||
dromo77 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
import logos from './demo/logos.json'; | ||
|
||
<Meta title="Objects/Logo Group" /> | ||
|
||
# Logo Group | ||
|
||
Logo Group can be used to display a group of client logos. Logos break onto multiple lines depending on the available space and are vertically centered. | ||
|
||
<Canvas> | ||
<Story name="Default">{defaultDemo({ items: logos })}</Story> | ||
dromo77 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
</Canvas> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
<div class="o-logo-group"> | ||
{% block content %}{% endblock %} | ||
</div> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the negative margin, curious what you think of it now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dromo77 Did you push that change? I still see the negative margin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No 😆 should be there now.