Skip to content
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

Style critical Gutenberg block markup #710

Closed
9 tasks done
tylersticka opened this issue May 18, 2020 · 9 comments
Closed
9 tasks done

Style critical Gutenberg block markup #710

tylersticka opened this issue May 18, 2020 · 9 comments
Labels
🎨 design Requires visual, UX or UI design decisions

Comments

@tylersticka
Copy link
Member

tylersticka commented May 18, 2020

The new version of our WordPress theme will support WordPress's Gutenberg editing experience.

We've already added markup examples for these blocks to Storybook (see #590). But some of these would benefit from some additional styles.

How should we do this?

  • 1. Create a new wordpress directory in src/third-party or src/vendor (depending on the outcome of Unify third-party and vendor directories #709).

  • 1. Create one or more CSS files within that directory that style the WordPress Gutenberg blocks using the classes they provide.

    • If we have an equivalent pattern, we should use Sass mixins to extend that pattern to the WordPress equivalent. For example, we could create a mixin that lets us apply the styles for c-button to wp-block-button__link.
    • If there is no equivalent pattern, we can author original styles.

Why not modify the markup instead?

Since our patterns already include classes and markup structures for some Gutenberg patterns, it's reasonable to ask if we can customize their output. But I'd like to avoid that strategy for a few reasons:

  • We tried modifying markup of shortcodes before, and those modifications ended up causing regressions when new features were added later on.
  • Our incentive at the time for modifying markup was that existing markup did not use semantic HTML5 elements. This is no longer the case.
  • Having this customization live mostly in CSS land gives more of the team an opportunity to contribute to their maintenance than if they were in PHP and React.
  • Repeating CSS via mixins has very low performance impact and is easier than ever now that we're using Sass again.

Shouldn't this be in our WordPress theme?

A solid argument could be made that we should be exposing necessary mixins via the npm package and applying those as needed within the WordPress theme instead of doing that work here.

But our pattern work is much farther along than our theme work, and we already have markup examples, so it seems silly to needlessly complicate our short-term process.

Priorities

There are currently 19 Gutenberg blocks we've documented in our Storybook. Of those, I think we should prioritize the following:

The other blocks are either (a) mostly working already or (b) seem less likely to be used in the short term.

Where possible, I'd prefer these be submitted as separate PRs instead of one big PR.

@tylersticka tylersticka added 🎨 design Requires visual, UX or UI design decisions 📐 needs size labels May 18, 2020
@derekshirk derekshirk added 🎨 design Requires visual, UX or UI design decisions and removed 📐 needs size 🎨 design Requires visual, UX or UI design decisions labels May 18, 2020
@haysjoey haysjoey added size:20 and removed size:10 labels May 26, 2020
@derekshirk
Copy link
Contributor

derekshirk commented Jun 4, 2020

👋 Hi @tylersticka,

I wanted to ask you if you had any particular vision for column styles that you would like to see added to the existing WP Gutenberg output.

Looking at the column output in Storybook we see the unstyled output. wp-block-columns has no associated styles, while its children wp-block-column elements receive inline flex-basis values.

Screen Shot 2020-06-04 at 11 43 25 AM

While testing the columns block in a local Cloud Four site, I see that the default WP asset block-library/style.css includes some (somewhat suitable seeming) column styles.

Screen Shot 2020-06-04 at 11 04 23 AM

wp-columns

From /wp-includes/css/dist/block-library/style.css:

.wp-block-columns {
  display: flex;
  margin-bottom: 28px;
  flex-wrap: wrap; 
}

@media (min-width: 782px) {
  .wp-block-columns {
      flex-wrap: nowrap; 
  } 
}

.wp-block-columns.has-background {
  padding: 20px 30px; 
}

.wp-block-column {
  flex-grow: 1;
  min-width: 0;
  word-break: break-word;
  overflow-wrap: break-word; 
}

@media (max-width: 599px) {
  .wp-block-column {
      flex-basis: 100% !important;
  }
}

@media (min-width: 600px) and (max-width: 781px) {
  .wp-block-column {
    flex-basis: calc(50% - 16px) !important;
    flex-grow: 0;
  }

  .wp-block-column:nth-child(even) {
    margin-left: 32px; 
  } 
}

@media (min-width: 782px) {
  .wp-block-column {
    flex-basis: 0;
    flex-grow: 1; 
  }
  .wp-block-column[style] {
    flex-grow: 0; 
  }
  .wp-block-column:not(:first-child) {
     margin-left: 32px; 
  } 
}

Were you hoping to see changes introduce other (better) CSS column techniques? CSS Grid with @supports, improved responsive styling, or layout and spacing that makes better use of our spacing design tokens?

@tylersticka
Copy link
Member Author

@derekshirk If the block works well without intervention from us, let's move on to the next block that will benefit from our attention.

I could spend time nitpicking the gutter inconsistencies and such, but I don't think that should be our priority now.

@tylersticka
Copy link
Member Author

@derekshirk Your screenshot made me realize that we don't seem to be accounting for the presence of WordPress's default block styles in our Storybook.

If you'd like to start doing that, here's how you could go about it...

  1. In Terminal: npm i -D @wordpress/block-library
  2. Then add below other imports in gutenberg.stories.mdx: import '@wordpress/block-library/build-style/style.css';

Now the stories will include the WordPress block library styles. If WordPress updates them, we'll be notified and can make adjustments as needed in our stories.

@tylersticka
Copy link
Member Author

(If you find those styles aren't being output in the right order, you could instead add the import to .storybook/preview.js before our src/index.scss import.)

@derekshirk
Copy link
Contributor

Thanks @tylersticka!

@derekshirk
Copy link
Contributor

derekshirk commented Jun 4, 2020

While working on #778, I noticed a visual discrepancy with the Outlined/Secondary Gutenberg block styles (Within Storybook).

When testing in my local.cloudfour.com environment, I enqueued the standalone.css styles from Storybook (as v-next.css) in WordPress, after our main pattern styles like this:

Screen Shot 2020-06-04 at 4 23 14 PM

This produces the following button presentation when viewing a post:

Screen Shot 2020-06-04 at 4 21 01 PM

However, in Storybook, (via #778) I imported the @wordpress/block-library styles within our gutenberg.stories.mdx file, which produces the following button presentation:

Screen Shot 2020-06-04 at 4 21 09 PM

Notice how the secondary button is black and not blue!

I also tried importing the @wordpress/block-library in the .storybook/preview.js before our src/index.scss import. However, this produced the same visual results - a black outline secondary button with black text.

I either need to modify the styles that were added in #729, or reevaluate how the WordPress styles are being loaded in #778.

@tylersticka
Copy link
Member Author

Weird!

You're welcome to break that off as a separate issue if it makes it easier to address separately.

@tylersticka
Copy link
Member Author

We should make sure the embed block supports the WordPress responsive aspect ratio feature: https://developer.wordpress.org/block-editor/developers/themes/theme-support/#responsive-embedded-content

@tylersticka
Copy link
Member Author

Closing this in favor of smaller issues for remaining work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎨 design Requires visual, UX or UI design decisions
Projects
None yet
Development

No branches or pull requests

3 participants