Skip to content

Update quote styles#1556

Merged
dromo77 merged 14 commits into
v-nextfrom
quote-styles
Oct 4, 2021
Merged

Update quote styles#1556
dromo77 merged 14 commits into
v-nextfrom
quote-styles

Conversation

@dromo77

@dromo77 dromo77 commented Sep 30, 2021

Copy link
Copy Markdown
Contributor

Overview

This PR updates the pull quote and block quotes styles. It uses the Gutenberg block classes to apply styles so that the blockquote element remains fairly unopinionated. It also applies those same styles to all blockquote elements within the c-comment component.

Screenshots

Screen Shot 2021-09-30 at 11 46 26 AM

Testing

  1. Deploy preview

@dromo77
dromo77 requested a review from a team September 30, 2021 18:56
@changeset-bot

changeset-bot Bot commented Sep 30, 2021

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 7bf2834

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@cloudfour/patterns Minor

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

@netlify

netlify Bot commented Sep 30, 2021

Copy link
Copy Markdown

✔️ 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

Comment on lines +90 to +101
.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';
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since these styles are repeated here and in our WordPress vendor styles, should we create a Sass mixin and/or design tokens for them?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's a good point, I think tokens would work well here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@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.

Comment thread src/components/comment/comment.scss Outdated
Comment on lines +91 to +92
border-left: size.$edge-medium solid color.$base-gray-light;
color: color.$base-gray-dark;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should include styles for our dark theme as well.

Comment on lines +472 to +474
<footer>
<cite>Matt Mullenweg</cite>
</footer>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Did you verify that WordPress includes the footer element? If not, then we probably can't modify this markup.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I didn't, I'll double check that.

@dromo77
dromo77 requested a review from tylersticka October 1, 2021 17:03
Comment thread src/components/comment/comment.scss Outdated
Comment on lines +13 to +21
@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};
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just curious if we could simplify by using an em dash directly instead of needing the \ code?

content: '—';

@dromo77
dromo77 requested a review from tylersticka October 1, 2021 20:31
Comment thread src/components/comment/comment.scss Outdated
/// those comments visually with their parent.
.c-comment.is-replying::after,
.c-comment--thread::after {
background-color: var(--theme-color-border-kbd);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

@dromo77
dromo77 requested a review from tylersticka October 1, 2021 21:00

@tylersticka tylersticka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great work, @dromo77! Thanks for weathering the rounds of feedback.

@dromo77
dromo77 merged commit c37e464 into v-next Oct 4, 2021
@dromo77
dromo77 deleted the quote-styles branch October 4, 2021 16:20
@github-actions github-actions Bot mentioned this pull request Oct 4, 2021
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.

Refine blog post block quote styling

2 participants