-
Notifications
You must be signed in to change notification settings - Fork 3
Add default avatar, clean up some design stories #1393
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
Conversation
🦋 Changeset detectedLatest commit: 52c0232 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 |
| // the `.svg.twig` file causes linting errors (since they are not tracked). | ||
| // Therefore, we must track a twig file to satisfy CI. | ||
| import brandLogo from './demo/logo.twig'; | ||
| import logoSrc from '../assets/brand/logo.svg'; |
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.
Rather than maintain a demo file just to display the image, I thought it made more sense to import the image source and display it via an img element in the story.
| Cloud Four logo in SVG format. The logo fill color can be modified via the CSS | ||
| `color` property. |
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 this text because it isn't accurate. It describes behavior of our Icon component, but there's nothing about the SVG asset on its own that inherits the color property.
|
|
||
| {% embed '@cloudfour/objects/deck/deck.twig' %} | ||
| {% embed '@cloudfour/objects/deck/deck.twig' with { | ||
| class: 'o-deck--2-column@l' |
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 class was previously specified via the story in args, but I thought that was confusing. Moving it here means there's only one file to edit for this demo.
Paul-Hebert
left a comment
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.
Looks good!
This will be leveraged by our WordPress theme along with our color tokens to dynamically generate fallback avatars where necessary
❤️
Overview
This PR adds our usual default avatar asset. This will be leveraged by our WordPress theme along with our color tokens to dynamically generate fallback avatars where necessary. In the future, we could expand on this with different foreground images, too, but I didn't want to get too much further in the weeds on this.
I was originally going to place this in the "Design ▸ Brand" stories alongside "Logo," until I realized it was really more of a fallback image. Along the way I ended up reorganizing and cleaning up some of those stories.
Screenshots
Testing
On the deploy preview, confirm that there are no regressions in design stories.