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

Cover: Add typography supports #43298

Merged
merged 1 commit into from
Aug 18, 2022
Merged

Conversation

aaronrobertshaw
Copy link
Contributor

@aaronrobertshaw aaronrobertshaw commented Aug 17, 2022

Depends on:

Related:

What?

Adds typography support to the Cover block.

Why?

  • Improves consistency of our design tools across blocks.
  • Allows some typographic styling to be set in a single place and applied to all the Cover's inner blocks.

How?

  • Opts into typography supports.

Testing Instructions

  1. Edit a post, add a Cover block with multiple paragraphs inside and select it.
  2. Test various typography settings ensuring styles are applied in the editor.
    • Note that by default the Cover block current applies font size to the first paragraph which will take precedence.
  3. Save and confirm the application on the frontend.
  4. Cherry pick the changes from Duotone: Prevent early return blocking other style generation #43300 to this branch
  5. Switch to the site editor and select a page/template with a Cover block.
  6. Navigate to Global Styles > Blocks > Cover > Typography and apply typography styles there.
  7. Confirm the selected styles are reflected in the preview and on the frontend.

Screenshots or screencast

Screen.Recording.2022-08-17.at.3.49.26.pm.mp4

@aaronrobertshaw aaronrobertshaw added [Type] Enhancement A suggestion for improvement. [Block] Cover Affects the Cover Block - used to display content laid over a background image [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Feature] Typography Font and typography-related issues and PRs labels Aug 17, 2022
@aaronrobertshaw aaronrobertshaw self-assigned this Aug 17, 2022
Copy link
Contributor

@andrewserong andrewserong left a comment

Choose a reason for hiding this comment

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

I noticed that adjusting these Typography controls also affects rendering in the placeholder state. This appears to happen in both the post editor at the individual block level, and when setting typography controls in global styles:

Post editor Global Styles
image image

I haven't looked closely, but I was wondering if we need to update the Edit component so that if it's in the placeholder state, we don't output the style rules? I imagine the global styles case might be trickier 🤔

Aside from the issue with the placeholder state, otherwise the controls are testing well for me in the post editor and global styles with #43300 applied 👍

@aaronrobertshaw
Copy link
Contributor Author

This appears to happen in both the post editor at the individual block level, and when setting typography controls in global styles

I'm wondering if we'll need a solution for placeholders in general. Any block using a placeholder and typography supports (among others e.g. border) will suffer the same issue.

Borders might be a special case, as for the Post Featured Image within a Query Loop in the site editor, there's a case to be made for displaying the border around the placeholder.

@talldan
Copy link
Contributor

talldan commented Aug 17, 2022

It's an issue that has been around for a while. After a quick look I found these issues:

I remember also briefly discussing it with @ajlende in an issue/PR. It's a difficult one to solve.

Probably two ways to go about it. Either not show particular block tools when the placeholder is visible, or try to prevent the styles from affecting the placeholder (popover/iframe/shadow dom). The problem with the latter option is that some styles need to be let through (otherwise something like this would be easy - https://matthewjamestaylor.com/style-blocker).

One idea might be to add a context provider around blocks that stores state around whether block supports features are visible/enabled. Then each placeholder could disable design tools when mounting, but enable when unmounting.

Copy link
Contributor

@tellthemachines tellthemachines left a comment

Choose a reason for hiding this comment

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

Works as expected!

@aaronrobertshaw
Copy link
Contributor Author

Given we'll look for a holistic solution to block placeholders inheriting the typography styles, I'll merge this one and follow-up on that separately.

@aaronrobertshaw aaronrobertshaw merged commit 5a028d1 into trunk Aug 18, 2022
@aaronrobertshaw aaronrobertshaw deleted the add/typography-supports-to-cover branch August 18, 2022 05:02
@github-actions github-actions bot added this to the Gutenberg 14.0 milestone Aug 18, 2022
@femkreations femkreations added the Needs User Documentation Needs new user documentation label Sep 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Cover Affects the Cover Block - used to display content laid over a background image [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Feature] Typography Font and typography-related issues and PRs Needs User Documentation Needs new user documentation [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants