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

List Item: Adopt typography supports #43312

Merged
merged 1 commit into from
Oct 18, 2022

Conversation

aaronrobertshaw
Copy link
Contributor

Related:

What?

Adds all typography support to the List Item block.

Why?

  • Improves consistency of our design tools across blocks.

How?

  • Opts into all typography support.

Testing Instructions

  1. Make sure you have the "List block v2" Gutenberg experiment turned on.
    • Screen Shot 2022-08-17 at 7 01 53 pm
  2. Edit a post, add a List block with multiple items, and select an item.
  3. Test various typography settings ensuring styles are applied in the editor.
  4. Save and confirm the application on the frontend.
  5. Switch to the site editor and add a List with items to a page or template.
  6. Navigate to Global Styles > Blocks > List Item > 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.6.59.05.pm.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 [Feature] Typography Font and typography-related issues and PRs labels Aug 17, 2022
@aaronrobertshaw aaronrobertshaw self-assigned this Aug 17, 2022
@glendaviesnz
Copy link
Contributor

This worked well in the post editor, but when I set the typography supports in global styles, on saving the changes are reverted - I wonder it is related to this being experimental?

list-item.mp4

@aaronrobertshaw
Copy link
Contributor Author

Thanks for catching that @glendaviesnz 👍

This worked well in the post editor, but when I set the typography supports in global styles, on saving the changes are reverted - I wonder it is related to this being experimental?

I'll dig into this later today. The experimental status of the ListItem block could perhaps mean it is filtered from the merged global styles. Given the experimental status, I don't think there's any rush in getting these supports in place for 6.1.

@glendaviesnz glendaviesnz force-pushed the add/typography-support-to-list-item branch from f878ba2 to ec93887 Compare October 12, 2022 22:31
@glendaviesnz
Copy link
Contributor

Rebased this, and even though the list v2 is not experimental anymore the list item typography supports don't save in global styles still.

@glendaviesnz
Copy link
Contributor

glendaviesnz commented Oct 12, 2022

The values are being serialised into the global styles custom post:

{"styles":{"blocks":{"core\/list-item":{"typography":{"fontFamily":"var:preset|font-family|source-serif-pro"}}}},"isGlobalStylesUserThemeJSON":true,"version":2}

but it looks like core/list-item is not returned as a valid block here, so the values are removed when the global styles are parsed.

I can't see anything obvious missing from the block registration that is causing this.

Should we just move this to the blocked column for now @aaronrobertshaw as not critical as the font family can at least be set on the parent?

@aaronrobertshaw
Copy link
Contributor Author

but it looks like core/list-item is not returned as a valid block here, so the values are removed when the global styles are parsed.

If I install the WP 6.1 RC, the core/list-item block shows up and everything looks to work. I believe that's because core would be registering the block so it is there from the start.

I wonder if this is another caching or timing related issue for theme.json processing? As in, this code is running before Gutenberg registers or re-registers its blocks.

Looks like #44658 could be related.

@glendaviesnz
Copy link
Contributor

If I install the WP 6.1 RC, the core/list-item block shows up and everything looks to work.

That is strange, in WP 6.1 RC1 the List Item block doesn't show in global styles for me:
Screen Shot 2022-10-13 at 3 09 58 PM

🤔

@aaronrobertshaw
Copy link
Contributor Author

That is strange, in WP 6.1 RC1 the List Item block doesn't show in global styles for me:

I think I found the issue here.

When the List Item block landed in core, it wasn't added to the list of blocks for Gutenberg to override the implementation of. When I add that there, then the List Item definitely shows in the Global Styles block, has the Gutenberg added block supports, and the styles generate, save & apply correctly.

I'll put up a PR to fix that later today.

@aaronrobertshaw aaronrobertshaw force-pushed the add/typography-support-to-list-item branch from ec93887 to 3cda4f2 Compare October 17, 2022 05:45
@aaronrobertshaw
Copy link
Contributor Author

I've rebased this PR to pick up the fix from #44911. Should be right for a final review ✅.

@glendaviesnz, I believe you've already tested this PR with the fix, is there anything else you think needs tweaking?

Copy link
Contributor

@glendaviesnz glendaviesnz left a comment

Choose a reason for hiding this comment

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

LGTM

@aaronrobertshaw aaronrobertshaw merged commit 8a5ccdf into trunk Oct 18, 2022
@aaronrobertshaw aaronrobertshaw deleted the add/typography-support-to-list-item branch October 18, 2022 00:05
@github-actions github-actions bot added this to the Gutenberg 14.4 milestone Oct 18, 2022
@femkreations femkreations added the Needs User Documentation Needs new user documentation label Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[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.

3 participants