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

Home Link: Add typography support #43307

Merged
merged 1 commit into from
Aug 26, 2022

Conversation

aaronrobertshaw
Copy link
Contributor

Related:

What?

Adds typography support to the Home Link block.

Why?

  • Improves consistency of our design tools across blocks.
  • Allows applying typographic styles to the Home link independent of other navigation links.

How?

  • Opts into typography supports.
  • Makes only the font size control displayed by default

Testing Instructions

  1. Edit a post, add a Navigation block with a menu including a Home Link.
  2. Select the Home Link block and confirm font size control displayed by default.
  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 select a page with a Navigation block including a Home Link.
  6. Navigate to Global Styles > Blocks > Home Link > 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.5.31.14.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] Home Link Affects the Home Link 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

The control works fine, but there are CSS specificity issues. Should this be adressed in another PR?

  1. Add a size to the navigation block.
  2. Add a different size to the home link.
  3. In the block editor, the sizes are correct.
  4. On the front, the size that is added to the navigation block is used.

A navigation block with two font sizes. The browser console is open and shows how the CSS rule for the navigation block overrides the font size for the home link

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.

Code looks good!
Tested on post and site editor as well as global styles and everything works fine. It's nice to be able to style Home Link differently from the regular links too:

Screen Shot 2022-08-17 at 6 06 42 pm

@@ -293,7 +293,7 @@ Create a link that always points to the homepage of the site. Usually not necess

- **Name:** core/home-link
- **Category:** design
- **Supports:** ~~html~~, ~~reusable~~
- **Supports:** typography (fontSize, lineHeight), ~~html~~, ~~reusable~~
Copy link
Contributor

Choose a reason for hiding this comment

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

So we don't seem to ever list the full set of typography options, right? I notice other blocks look the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the output from the automatic doc generation. I believe it doesn't include any experimental APIs.

One of the later phases of the design tool consistency effort will be to stabilize our block support APIs. These docs will be updated again we that happens.

@tellthemachines
Copy link
Contributor

@carolinan that's an interesting problem! (Also thanks for your comprehensive testing 😄 )

The classname for the Navigation-level styles is being added to each li in the block's index.php, before the block support adds its own class. I'm not sure why we need to add the class to each li in the front end, because in the editor we just add it to the Navigation wrapper. That would be worth looking into separately, I reckon.

@aaronrobertshaw
Copy link
Contributor Author

That would be worth looking into separately, I reckon.

Agreed. Investigating via a separate follow-up looking specifically at the Navigation block sounds like a plan.

I'll add a note about the follow-up to the tracking issue and then merge this.

@aaronrobertshaw aaronrobertshaw merged commit fc372b5 into trunk Aug 26, 2022
@aaronrobertshaw aaronrobertshaw deleted the add/typography-support-to-home-link branch August 26, 2022 07:47
@github-actions github-actions bot added this to the Gutenberg 14.1 milestone Aug 26, 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] Home Link Affects the Home Link 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.

4 participants