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

Page List: Add typography supports #43316

Merged
merged 1 commit into from
Dec 15, 2022

Conversation

aaronrobertshaw
Copy link
Contributor

Related:

What?

Adds all typography support to the Page List block.

Why?

  • Improves consistency of our design tools across blocks.
  • Provides a means of applying typographic styles for the Page List.

How?

  • Opts into all typography support.
  • Makes the font size control display by default.

Testing Instructions

  1. Edit a post, add a Page List block, and select it.
  2. Test various typography settings ensuring styles are applied in the editor.
  3. Save and confirm the application on the frontend.
  4. Switch to the site editor and add a Page List to a page or template.
  5. Navigate to Global Styles > Blocks > Page List > Typography and apply typography styles there.
  6. Confirm the selected styles are reflected in the preview and on the frontend.

Screenshots or screencast

Screen.Recording.2022-08-17.at.7.48.45.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 [Block] Page List Affects the Page List Block [Feature] Typography Font and typography-related issues and PRs labels Aug 17, 2022
@aaronrobertshaw aaronrobertshaw self-assigned this Aug 17, 2022
@carolinan
Copy link
Contributor

With the exception of the text decoration underline, which we are aware of does not work:

  • As a standalone block: Works well.

  • When part of the navigation block: Compared to trunk, there seems to be an issue with the padding.

Trunk:
Screen Shot 2022-08-26 at 12 49 15

Trunk, with large font size and bold set on the navigation block, for comparison:
Screen Shot 2022-08-26 at 12 52 08

PR:
Screen Shot 2022-08-26 at 12 43 18

@glendaviesnz
Copy link
Contributor

I retested this and it all seems to be working as expected, but I wasn't able to replicate your setup with the navigation block @carolinan - how do you add a Page list block as a dropdown in a Nav menu the way you have it?

@carolinan
Copy link
Contributor

carolinan commented Sep 13, 2022

Create a few pages with and without child pages on the test install. Or import the theme test data.

In the block editor or site editor, add a navigation block.

Select the navigation block.
Click Add Block in the editor canvas: A custom link is added to the navigation. This block needs to be selected.
Now toggle the block inserter in the top left corner.

The block inserter should show a reduced number of items:
Block inserter for the navigation block

Select the page list block. It should now be visible both in the editor and in the list view, as an inner block of the navigation block.

Hover over a page with a child page to see the spacing:
Submenus in page list is missing spacing

@glendaviesnz
Copy link
Contributor

🤦 ! thanks @carolinan, was overlooking the good old-fashioned parent page option!

I was able to replicate the same issue you are having with padding disappearing on the dropdown menu page list on this branch - works as expected on trunk. Will try and take a closer look at why sometime this week.

@carolinan
Copy link
Contributor

It happens even if I do not make any changes to the typography settings 🤔

@glendaviesnz glendaviesnz force-pushed the add/typography-support-to-page-list branch from 27e43de to 62b950c Compare October 12, 2022 23:45
@glendaviesnz
Copy link
Contributor

@carolinan I rebased this and retested it and I am not seeing the lack of padding issue now:

Screen Shot 2022-10-13 at 12 44 33 PM

are you able to give it another go please when you have a minute please and see if it is resolved for you as well now?

Copy link
Contributor

@carolinan carolinan left a comment

Choose a reason for hiding this comment

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

In block themes (Twenty Twenty-Three, empty theme): The spacing is correct in the editor and front in the standalone block, the navigation block, and navigation block overlay.

Not blockers:

Twenty Nineteen:
The spacing is correct in the editor and front.
The exception is on the front, in the navigation block overlay, where the spacing is exceptionally large and a 2em blockgap is applied. This is unrelated to this PR.

Twenty Sixteen:
The page list is broken when placed in the navigation block: there is too much spacing between the submenus, so the "show menu items on hover" stops working. I believe this is a theme issue unrelated to this PR.

@aaronrobertshaw aaronrobertshaw force-pushed the add/typography-support-to-page-list branch from c1d5f80 to f653e97 Compare December 15, 2022 10:38
@aaronrobertshaw aaronrobertshaw merged commit 9c12246 into trunk Dec 15, 2022
@aaronrobertshaw aaronrobertshaw deleted the add/typography-support-to-page-list branch December 15, 2022 11:55
@github-actions github-actions bot added this to the Gutenberg 14.9 milestone Dec 15, 2022
dmsnell pushed a commit that referenced this pull request Dec 15, 2022
Co-authored-by: Glen Davies <glen.davies@automattic.com>
@femkreations femkreations added the Needs User Documentation Needs new user documentation label Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Page List Affects the Page List 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.

None yet

4 participants