Update quote styles#1556
Conversation
🦋 Changeset detectedLatest commit: 7bf2834 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 |
|
✔️ Deploy Preview for cloudfour-patterns ready! 🔨 Explore the source changes: 7bf2834 🔍 Inspect the deploy log: https://app.netlify.com/sites/cloudfour-patterns/deploys/6157758bd0510400082f11da 😎 Browse the preview: https://deploy-preview-1556--cloudfour-patterns.netlify.app |
| .c-comment__content blockquote { | ||
| border-left: size.$edge-medium solid color.$base-gray-light; | ||
| color: color.$base-gray-dark; | ||
| } | ||
|
|
||
| .c-comment__content blockquote cite { | ||
| font-style: normal; | ||
| } | ||
|
|
||
| .c-comment__content blockquote cite::before { | ||
| content: '\2014'; | ||
| } |
There was a problem hiding this comment.
Since these styles are repeated here and in our WordPress vendor styles, should we create a Sass mixin and/or design tokens for them?
There was a problem hiding this comment.
That's a good point, I think tokens would work well here.
There was a problem hiding this comment.
@tylersticka I added tokens for most of these styles. However, I'm having trouble using a token for font-style: normal. I added a token in tokens/font/styles.json, but I get an error when I try to use it.
| border-left: size.$edge-medium solid color.$base-gray-light; | ||
| color: color.$base-gray-dark; |
There was a problem hiding this comment.
We should include styles for our dark theme as well.
| <footer> | ||
| <cite>Matt Mullenweg</cite> | ||
| </footer> |
There was a problem hiding this comment.
Did you verify that WordPress includes the footer element? If not, then we probably can't modify this markup.
There was a problem hiding this comment.
I didn't, I'll double check that.
| @include theme.props() { | ||
| --theme-color-border-blockquote: #{color.$border-blockquote-light}; | ||
| --theme-color-text-blockquote: #{color.$text-blockquote-light}; | ||
| } | ||
|
|
||
| @include theme.props(dark) { | ||
| --theme-color-border-blockquote: #{color.$border-blockquote-dark}; | ||
| --theme-color-text-blockquote: #{color.$text-blockquote-dark}; | ||
| } |
There was a problem hiding this comment.
Reading this now, I think for the text you might be able to use --theme-color-text-muted instead of defining new properties and tokens? See src/base/_themes.scss
I could see adding a new border property in the base themes as well, esp. since we're wanting to coordinate that color with comment threads.
There was a problem hiding this comment.
Cool! I didn't know we had theme tokens. I updated the color and border color properties so that we're using existing tokens.
| } | ||
|
|
||
| .c-comment__content blockquote cite::before { | ||
| @include emdash.content; |
There was a problem hiding this comment.
Just curious if we could simplify by using an em dash directly instead of needing the \ code?
content: '—';
| /// those comments visually with their parent. | ||
| .c-comment.is-replying::after, | ||
| .c-comment--thread::after { | ||
| background-color: var(--theme-color-border-kbd); |
There was a problem hiding this comment.
Can we create a border property specifically for this? kbd refers to the kbd element, so I'm not sure it's the most intuitive thing to use here. https://developer.mozilla.org/en-US/docs/Web/HTML/Element/kbd
If it happens to be the right color, maybe we just need to rename it wherever it's referenced to something more general.
There was a problem hiding this comment.
I'm not sure it's the most intuitive thing to use here
Haha probably not 😆. I added a property specifically for the border color.
tylersticka
left a comment
There was a problem hiding this comment.
Great work, @dromo77! Thanks for weathering the rounds of feedback.
Overview
This PR updates the pull quote and block quotes styles. It uses the Gutenberg block classes to apply styles so that the
blockquoteelement remains fairly unopinionated. It also applies those same styles to allblockquoteelements within thec-commentcomponent.Screenshots
Testing