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

Post Content: Add typography supports #43339

Merged
merged 1 commit into from
Sep 6, 2022

Conversation

aaronrobertshaw
Copy link
Contributor

Related:

What?

Adds typography supports to the Post Content block.

Why?

  • Improves consistency of our design tools across blocks.
  • Provides high-level typographic styling for post content.

How?

  • Opts into all typography supports.
  • Makes font size control displayed by default as per majority of blocks currently.

Testing Instructions

  1. In the site editor, edit a template with a Post Content block.
  2. Navigate to Global Styles > Blocks > Post Content > Typography.
  3. Experiment with typography styles and ensure they are applied in the preview.
  4. Confirm styles on the frontend.
  5. Reset Global Styles to defaults, select the Post Content block within the preview.
  6. On the Block tab of the Settings sidebar, test the typography controls for the individual block.
  7. Confirm these work on the frontend as well.

Screenshots or screencast

Screen.Recording.2022-08-18.at.10.12.42.am.mp4

@aaronrobertshaw aaronrobertshaw added [Type] Enhancement A suggestion for improvement. [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Block] Post Content Affects the Post Content Block [Feature] Typography Font and typography-related issues and PRs labels Aug 18, 2022
@aaronrobertshaw aaronrobertshaw self-assigned this Aug 18, 2022
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.

This works fine on the front end, but it's not showing the changes in the post editor. Ideally, we'd be able to preview the actual styles when editing a post.

I don't think there's a way for the post editor to know about the settings of the Post Content block, but it would be useful for things like content size too. I guess it's something to look into.

@aaronrobertshaw
Copy link
Contributor Author

aaronrobertshaw commented Aug 18, 2022

Thanks for taking the time to review this one @tellthemachines 🙇

I don't think there's a way for the post editor to know about the settings of the Post Content block, but it would be useful for things like content size too. I guess it's something to look into.

It's a bit of a tricky one. Within the site editor, you could configure settings on an individual Post Content block for a particular template. That wouldn't apply in all cases, so not sure if that means we can safely ignore that for now.

If the Post Content block is styled via Global Styles and we added the .wp-block-post-content class to the root BlockList, then it gets those global typography styles applied. I have no idea about the consequences of doing that at the moment.

Example diff just to get class applied and see if styles got picked up in the post editor.
diff --git a/packages/edit-post/src/components/visual-editor/index.js b/packages/edit-post/src/components/visual-editor/index.js
index 0d8614946a..b81c56b8f8 100644
--- a/packages/edit-post/src/components/visual-editor/index.js
+++ b/packages/edit-post/src/components/visual-editor/index.js
@@ -265,7 +265,7 @@ export default function VisualEditor( { styles } ) {
 								className={
 									isTemplateMode
 										? 'wp-site-blocks'
-										: 'is-layout-flow' // Ensure root level blocks receive default/flow blockGap styling rules.
+										: 'is-layout-flow wp-block-post-content' // Ensure root level blocks receive default/flow blockGap styling rules.
 								}
 								__experimentalLayout={ layout }
 							/>

@tellthemachines
Copy link
Contributor

Yeah, it doesn't seem to have negative consequences. I wouldn't expect there to be any styles attached to that block from our side at least. Might there be any scenario in which a theme would attach a style to that classname and not expect it to display in the editor?

In any case, that would only solve half the problem because it's just as likely settings would be changed in the actual block as opposed to global styles, and there might be more than one instance of the block with different settings.

I'm not sure what's best here. Perhaps we just add this in for now and then think of a good way for the post editor to reflect styles from the block 🤷

@carolinan
Copy link
Contributor

@WordPress/block-themers Any feedback on the discussions above about styling the post content block?

@MaggieCabrera
Copy link
Contributor

This reminded me of #38862 I don't know if solving the connection between the post content block and what shows in the post editor should be done on this PR or not, but it's definitely something that needs to be considered at some point.

@scruffian
Copy link
Contributor

Perhaps we just add this in for now and then think of a good way for the post editor to reflect styles from the block 🤷

I'm not sure this is a good idea. It will mean that we have discrepancies between the front/back end. I think we need to find a way to render content in the post editor inside a post content block before we can do it.

@carolinan
Copy link
Contributor

An example of where there already is a discrepancy between the front/back end is the link color in Twenty Twenty-Three, where all links in the post content block have a color on the front, that is not visible in the block editor.

@scruffian
Copy link
Contributor

I see that as more reason to fix the existing issues rather than creating more :)

@matiasbenedetto
Copy link
Contributor

matiasbenedetto commented Sep 5, 2022

I submitted this PR a few months ago aimed to fix the fact that style settings from core/post-content block in theme.json are not applied in the editor. It seems to be working fine with most of the style settings but I tried it in conjunction with these changes, and the editor is still failing to display the right font sizes because the properties and class names around font sizing are missing in the BlockList component.

@aaronrobertshaw aaronrobertshaw force-pushed the add/typography-support-to-post-content branch from 65f5c65 to 06141ee Compare September 5, 2022 23:18
@aaronrobertshaw
Copy link
Contributor Author

Thanks everyone for the discussion on this 👍

I've rebased this PR to pick up the latest changes from #42270 which from my testing does allow theme.json and global styles for the Post Content block to take effect within the block editor.

Summary:

  • If an individual Post Content block for a single post page is configured with block supports, those styles are not applied within the block editor.
  • Theme.json and Global Styles are applied correctly now post #42270 as it applies the .wp-block-post-content classname in the block editor.

Options

  1. Abandon adopting block supports for the Post Content block:
    • Pro: Doesn't introduce inconsistency between site editor, block editor, and frontend (albeit for a single template?)
    • Con: Decreases design tool consistency across blocks.
    • Con: Doesn't allow for users to override anything set by a theme author via theme.json
    • Con: Doesn't move toward a solution
  2. Merge as is:
    • Pro: Given theme.json will style the block, this allows a user to tweak those values via Global Styles.
    • Pro: Allows for individual Post Content blocks to be styled differently to the main single template if added to non-single-post templates
    • Con: Introduces a potentially limited inconsistency between site editor, block editor and the frontend.
    • Con: Requires further follow-up to address inconsistency which might not land before the initial 6.1 feature freeze.

My vote would be for Option 2.

Am I missing anything? What do you all think?

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.

Re-tested the changes and can confirm typography settings in global styles are now reflected in the post editor ✅

I'm inclined to merge this as it adds useful functionality, and the post editor not reflecting the styles added at the block level seems preferable to not being able to change typography settings at all.

We will have to address the issue of reflecting block-specific styles in the post editor at some point, as there are already discrepancies in the layout support, but it's highly unlikely that we'll get that in before Beta 1.

@aristath aristath merged commit ce4feda into trunk Sep 6, 2022
@aristath aristath deleted the add/typography-support-to-post-content branch September 6, 2022 07:29
@github-actions github-actions bot added this to the Gutenberg 14.1 milestone Sep 6, 2022
@femkreations femkreations added the Needs User Documentation Needs new user documentation label Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Post Content Affects the Post Content Block [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.

8 participants