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 Title Block: add typography formatting options #31623

Merged
merged 2 commits into from
Sep 15, 2021

Conversation

creativecoder
Copy link
Contributor

@creativecoder creativecoder commented May 8, 2021

Description

Adds text formatting options to the Post Title block using experimental block supports declarations.

Summary of changes

  • Adds font weight, font style, and text transform typography controls to the Post Title block.

Partially addresses #31117, but leaves out text decoration, for now

How has this been tested?

  • Install and activate the tt1-blocks theme
  • Open a template in the Site Editor that uses a Post Title block, such as Index or Single
  • Test the additional typography settings for the block. Note that they apply to the entire block contents, they are not inline controls
  • Make sure they work as expected with "Make title a link" on and off

Screenshots

Screen Shot 2021-09-14 at 14 38 55

Types of changes

New feature (non-breaking change which adds functionality)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

@creativecoder creativecoder added [Type] Enhancement A suggestion for improvement. [Block] Post Title Affects the Post Title Block labels May 8, 2021
@creativecoder creativecoder self-assigned this May 8, 2021
@github-actions
Copy link

github-actions bot commented May 8, 2021

Size Change: +157 B (0%)

Total Size: 1.06 MB

Filename Size Change
build/block-editor/index.min.js 128 kB +83 B (0%)
build/block-library/blocks/post-title/style-rtl.css 83 B +23 B (+38%) 🚨
build/block-library/blocks/post-title/style.css 83 B +23 B (+38%) 🚨
build/block-library/index.min.js 153 kB +21 B (0%)
build/block-library/style-rtl.css 10.3 kB +4 B (0%)
build/block-library/style.css 10.3 kB +3 B (0%)
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 931 B
build/admin-manifest/index.min.js 1.09 kB
build/annotations/index.min.js 2.7 kB
build/api-fetch/index.min.js 2.19 kB
build/autop/index.min.js 2.08 kB
build/blob/index.min.js 459 B
build/block-directory/index.min.js 6.2 kB
build/block-directory/style-rtl.css 1.01 kB
build/block-directory/style.css 1.01 kB
build/block-editor/default-editor-styles-rtl.css 378 B
build/block-editor/default-editor-styles.css 378 B
build/block-editor/style-rtl.css 13.8 kB
build/block-editor/style.css 13.8 kB
build/block-library/blocks/archives/editor-rtl.css 61 B
build/block-library/blocks/archives/editor.css 60 B
build/block-library/blocks/archives/style-rtl.css 65 B
build/block-library/blocks/archives/style.css 65 B
build/block-library/blocks/audio/editor-rtl.css 58 B
build/block-library/blocks/audio/editor.css 58 B
build/block-library/blocks/audio/style-rtl.css 111 B
build/block-library/blocks/audio/style.css 111 B
build/block-library/blocks/audio/theme-rtl.css 125 B
build/block-library/blocks/audio/theme.css 125 B
build/block-library/blocks/block/editor-rtl.css 161 B
build/block-library/blocks/block/editor.css 161 B
build/block-library/blocks/button/editor-rtl.css 474 B
build/block-library/blocks/button/editor.css 474 B
build/block-library/blocks/button/style-rtl.css 600 B
build/block-library/blocks/button/style.css 600 B
build/block-library/blocks/buttons/editor-rtl.css 315 B
build/block-library/blocks/buttons/editor.css 315 B
build/block-library/blocks/buttons/style-rtl.css 370 B
build/block-library/blocks/buttons/style.css 370 B
build/block-library/blocks/calendar/style-rtl.css 207 B
build/block-library/blocks/calendar/style.css 207 B
build/block-library/blocks/categories/editor-rtl.css 84 B
build/block-library/blocks/categories/editor.css 83 B
build/block-library/blocks/categories/style-rtl.css 79 B
build/block-library/blocks/categories/style.css 79 B
build/block-library/blocks/code/style-rtl.css 90 B
build/block-library/blocks/code/style.css 90 B
build/block-library/blocks/code/theme-rtl.css 131 B
build/block-library/blocks/code/theme.css 131 B
build/block-library/blocks/columns/editor-rtl.css 206 B
build/block-library/blocks/columns/editor.css 205 B
build/block-library/blocks/columns/style-rtl.css 497 B
build/block-library/blocks/columns/style.css 496 B
build/block-library/blocks/cover/editor-rtl.css 666 B
build/block-library/blocks/cover/editor.css 670 B
build/block-library/blocks/cover/style-rtl.css 1.23 kB
build/block-library/blocks/cover/style.css 1.23 kB
build/block-library/blocks/embed/editor-rtl.css 488 B
build/block-library/blocks/embed/editor.css 488 B
build/block-library/blocks/embed/style-rtl.css 417 B
build/block-library/blocks/embed/style.css 417 B
build/block-library/blocks/embed/theme-rtl.css 124 B
build/block-library/blocks/embed/theme.css 124 B
build/block-library/blocks/file/editor-rtl.css 300 B
build/block-library/blocks/file/editor.css 300 B
build/block-library/blocks/file/style-rtl.css 255 B
build/block-library/blocks/file/style.css 255 B
build/block-library/blocks/file/view.min.js 322 B
build/block-library/blocks/freeform/editor-rtl.css 2.44 kB
build/block-library/blocks/freeform/editor.css 2.44 kB
build/block-library/blocks/gallery/editor-rtl.css 983 B
build/block-library/blocks/gallery/editor.css 988 B
build/block-library/blocks/gallery/style-rtl.css 1.6 kB
build/block-library/blocks/gallery/style.css 1.59 kB
build/block-library/blocks/gallery/theme-rtl.css 122 B
build/block-library/blocks/gallery/theme.css 122 B
build/block-library/blocks/group/editor-rtl.css 159 B
build/block-library/blocks/group/editor.css 159 B
build/block-library/blocks/group/style-rtl.css 57 B
build/block-library/blocks/group/style.css 57 B
build/block-library/blocks/group/theme-rtl.css 70 B
build/block-library/blocks/group/theme.css 70 B
build/block-library/blocks/heading/style-rtl.css 114 B
build/block-library/blocks/heading/style.css 114 B
build/block-library/blocks/home-link/style-rtl.css 247 B
build/block-library/blocks/home-link/style.css 247 B
build/block-library/blocks/html/editor-rtl.css 332 B
build/block-library/blocks/html/editor.css 333 B
build/block-library/blocks/image/editor-rtl.css 728 B
build/block-library/blocks/image/editor.css 728 B
build/block-library/blocks/image/style-rtl.css 482 B
build/block-library/blocks/image/style.css 487 B
build/block-library/blocks/image/theme-rtl.css 124 B
build/block-library/blocks/image/theme.css 124 B
build/block-library/blocks/latest-comments/style-rtl.css 284 B
build/block-library/blocks/latest-comments/style.css 284 B
build/block-library/blocks/latest-posts/editor-rtl.css 137 B
build/block-library/blocks/latest-posts/editor.css 137 B
build/block-library/blocks/latest-posts/style-rtl.css 528 B
build/block-library/blocks/latest-posts/style.css 527 B
build/block-library/blocks/list/style-rtl.css 94 B
build/block-library/blocks/list/style.css 94 B
build/block-library/blocks/media-text/editor-rtl.css 266 B
build/block-library/blocks/media-text/editor.css 263 B
build/block-library/blocks/media-text/style-rtl.css 488 B
build/block-library/blocks/media-text/style.css 485 B
build/block-library/blocks/more/editor-rtl.css 431 B
build/block-library/blocks/more/editor.css 431 B
build/block-library/blocks/navigation-link/editor-rtl.css 489 B
build/block-library/blocks/navigation-link/editor.css 488 B
build/block-library/blocks/navigation-link/style-rtl.css 94 B
build/block-library/blocks/navigation-link/style.css 94 B
build/block-library/blocks/navigation-submenu/editor-rtl.css 300 B
build/block-library/blocks/navigation-submenu/editor.css 299 B
build/block-library/blocks/navigation-submenu/style-rtl.css 298 B
build/block-library/blocks/navigation-submenu/style.css 298 B
build/block-library/blocks/navigation-submenu/view.min.js 343 B
build/block-library/blocks/navigation/editor-rtl.css 1.72 kB
build/block-library/blocks/navigation/editor.css 1.72 kB
build/block-library/blocks/navigation/style-rtl.css 1.44 kB
build/block-library/blocks/navigation/style.css 1.44 kB
build/block-library/blocks/navigation/view.min.js 2.52 kB
build/block-library/blocks/nextpage/editor-rtl.css 395 B
build/block-library/blocks/nextpage/editor.css 395 B
build/block-library/blocks/page-list/editor-rtl.css 310 B
build/block-library/blocks/page-list/editor.css 310 B
build/block-library/blocks/page-list/style-rtl.css 241 B
build/block-library/blocks/page-list/style.css 241 B
build/block-library/blocks/paragraph/editor-rtl.css 157 B
build/block-library/blocks/paragraph/editor.css 157 B
build/block-library/blocks/paragraph/style-rtl.css 273 B
build/block-library/blocks/paragraph/style.css 273 B
build/block-library/blocks/post-author/editor-rtl.css 210 B
build/block-library/blocks/post-author/editor.css 210 B
build/block-library/blocks/post-author/style-rtl.css 182 B
build/block-library/blocks/post-author/style.css 181 B
build/block-library/blocks/post-comments-form/style-rtl.css 140 B
build/block-library/blocks/post-comments-form/style.css 140 B
build/block-library/blocks/post-comments/style-rtl.css 360 B
build/block-library/blocks/post-comments/style.css 359 B
build/block-library/blocks/post-content/editor-rtl.css 138 B
build/block-library/blocks/post-content/editor.css 138 B
build/block-library/blocks/post-excerpt/editor-rtl.css 73 B
build/block-library/blocks/post-excerpt/editor.css 73 B
build/block-library/blocks/post-excerpt/style-rtl.css 69 B
build/block-library/blocks/post-excerpt/style.css 69 B
build/block-library/blocks/post-featured-image/editor-rtl.css 398 B
build/block-library/blocks/post-featured-image/editor.css 398 B
build/block-library/blocks/post-featured-image/style-rtl.css 143 B
build/block-library/blocks/post-featured-image/style.css 143 B
build/block-library/blocks/post-template/editor-rtl.css 99 B
build/block-library/blocks/post-template/editor.css 98 B
build/block-library/blocks/post-template/style-rtl.css 378 B
build/block-library/blocks/post-template/style.css 379 B
build/block-library/blocks/post-terms/style-rtl.css 73 B
build/block-library/blocks/post-terms/style.css 73 B
build/block-library/blocks/preformatted/style-rtl.css 103 B
build/block-library/blocks/preformatted/style.css 103 B
build/block-library/blocks/pullquote/editor-rtl.css 198 B
build/block-library/blocks/pullquote/editor.css 198 B
build/block-library/blocks/pullquote/style-rtl.css 378 B
build/block-library/blocks/pullquote/style.css 378 B
build/block-library/blocks/pullquote/theme-rtl.css 167 B
build/block-library/blocks/pullquote/theme.css 167 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B
build/block-library/blocks/query-pagination/editor-rtl.css 301 B
build/block-library/blocks/query-pagination/editor.css 292 B
build/block-library/blocks/query-pagination/style-rtl.css 259 B
build/block-library/blocks/query-pagination/style.css 257 B
build/block-library/blocks/query-title/editor-rtl.css 85 B
build/block-library/blocks/query-title/editor.css 85 B
build/block-library/blocks/query/editor-rtl.css 131 B
build/block-library/blocks/query/editor.css 132 B
build/block-library/blocks/quote/style-rtl.css 187 B
build/block-library/blocks/quote/style.css 187 B
build/block-library/blocks/quote/theme-rtl.css 220 B
build/block-library/blocks/quote/theme.css 222 B
build/block-library/blocks/rss/editor-rtl.css 202 B
build/block-library/blocks/rss/editor.css 204 B
build/block-library/blocks/rss/style-rtl.css 289 B
build/block-library/blocks/rss/style.css 288 B
build/block-library/blocks/search/editor-rtl.css 165 B
build/block-library/blocks/search/editor.css 165 B
build/block-library/blocks/search/style-rtl.css 374 B
build/block-library/blocks/search/style.css 375 B
build/block-library/blocks/search/theme-rtl.css 64 B
build/block-library/blocks/search/theme.css 64 B
build/block-library/blocks/separator/editor-rtl.css 99 B
build/block-library/blocks/separator/editor.css 99 B
build/block-library/blocks/separator/style-rtl.css 250 B
build/block-library/blocks/separator/style.css 250 B
build/block-library/blocks/separator/theme-rtl.css 172 B
build/block-library/blocks/separator/theme.css 172 B
build/block-library/blocks/shortcode/editor-rtl.css 474 B
build/block-library/blocks/shortcode/editor.css 474 B
build/block-library/blocks/site-logo/editor-rtl.css 462 B
build/block-library/blocks/site-logo/editor.css 464 B
build/block-library/blocks/site-logo/style-rtl.css 153 B
build/block-library/blocks/site-logo/style.css 153 B
build/block-library/blocks/site-tagline/editor-rtl.css 86 B
build/block-library/blocks/site-tagline/editor.css 86 B
build/block-library/blocks/site-title/editor-rtl.css 84 B
build/block-library/blocks/site-title/editor.css 84 B
build/block-library/blocks/social-link/editor-rtl.css 165 B
build/block-library/blocks/social-link/editor.css 165 B
build/block-library/blocks/social-links/editor-rtl.css 812 B
build/block-library/blocks/social-links/editor.css 811 B
build/block-library/blocks/social-links/style-rtl.css 1.3 kB
build/block-library/blocks/social-links/style.css 1.3 kB
build/block-library/blocks/spacer/editor-rtl.css 307 B
build/block-library/blocks/spacer/editor.css 307 B
build/block-library/blocks/spacer/style-rtl.css 48 B
build/block-library/blocks/spacer/style.css 48 B
build/block-library/blocks/table/editor-rtl.css 471 B
build/block-library/blocks/table/editor.css 472 B
build/block-library/blocks/table/style-rtl.css 481 B
build/block-library/blocks/table/style.css 481 B
build/block-library/blocks/table/theme-rtl.css 188 B
build/block-library/blocks/table/theme.css 188 B
build/block-library/blocks/tag-cloud/style-rtl.css 146 B
build/block-library/blocks/tag-cloud/style.css 146 B
build/block-library/blocks/template-part/editor-rtl.css 636 B
build/block-library/blocks/template-part/editor.css 635 B
build/block-library/blocks/template-part/theme-rtl.css 101 B
build/block-library/blocks/template-part/theme.css 101 B
build/block-library/blocks/term-description/editor-rtl.css 90 B
build/block-library/blocks/term-description/editor.css 90 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B
build/block-library/blocks/text-columns/editor.css 95 B
build/block-library/blocks/text-columns/style-rtl.css 166 B
build/block-library/blocks/text-columns/style.css 166 B
build/block-library/blocks/verse/style-rtl.css 87 B
build/block-library/blocks/verse/style.css 87 B
build/block-library/blocks/video/editor-rtl.css 571 B
build/block-library/blocks/video/editor.css 572 B
build/block-library/blocks/video/style-rtl.css 173 B
build/block-library/blocks/video/style.css 173 B
build/block-library/blocks/video/theme-rtl.css 124 B
build/block-library/blocks/video/theme.css 124 B
build/block-library/common-rtl.css 853 B
build/block-library/common.css 849 B
build/block-library/editor-rtl.css 9.69 kB
build/block-library/editor.css 9.68 kB
build/block-library/reset-rtl.css 527 B
build/block-library/reset.css 527 B
build/block-library/theme-rtl.css 658 B
build/block-library/theme.css 663 B
build/block-serialization-default-parser/index.min.js 1.09 kB
build/block-serialization-spec-parser/index.min.js 2.79 kB
build/blocks/index.min.js 46.9 kB
build/components/index.min.js 209 kB
build/components/style-rtl.css 15.8 kB
build/components/style.css 15.8 kB
build/compose/index.min.js 10.2 kB
build/core-data/index.min.js 12.3 kB
build/customize-widgets/index.min.js 11.1 kB
build/customize-widgets/style-rtl.css 1.5 kB
build/customize-widgets/style.css 1.49 kB
build/data-controls/index.min.js 614 B
build/data/index.min.js 7.1 kB
build/date/index.min.js 31.5 kB
build/deprecated/index.min.js 428 B
build/dom-ready/index.min.js 304 B
build/dom/index.min.js 4.45 kB
build/edit-navigation/index.min.js 16.2 kB
build/edit-navigation/style-rtl.css 3.62 kB
build/edit-navigation/style.css 3.62 kB
build/edit-post/classic-rtl.css 492 B
build/edit-post/classic.css 494 B
build/edit-post/index.min.js 29 kB
build/edit-post/style-rtl.css 7.2 kB
build/edit-post/style.css 7.2 kB
build/edit-site/index.min.js 26.6 kB
build/edit-site/style-rtl.css 5.07 kB
build/edit-site/style.css 5.07 kB
build/edit-widgets/index.min.js 16.1 kB
build/edit-widgets/style-rtl.css 4.06 kB
build/edit-widgets/style.css 4.06 kB
build/editor/index.min.js 37.7 kB
build/editor/style-rtl.css 3.74 kB
build/editor/style.css 3.73 kB
build/element/index.min.js 3.17 kB
build/escape-html/index.min.js 517 B
build/format-library/index.min.js 5.34 kB
build/format-library/style-rtl.css 668 B
build/format-library/style.css 670 B
build/hooks/index.min.js 1.55 kB
build/html-entities/index.min.js 424 B
build/i18n/index.min.js 3.6 kB
build/is-shallow-equal/index.min.js 501 B
build/keyboard-shortcuts/index.min.js 1.72 kB
build/keycodes/index.min.js 1.3 kB
build/list-reusable-blocks/index.min.js 1.85 kB
build/list-reusable-blocks/style-rtl.css 838 B
build/list-reusable-blocks/style.css 838 B
build/media-utils/index.min.js 2.88 kB
build/notices/index.min.js 845 B
build/nux/index.min.js 2.03 kB
build/nux/style-rtl.css 747 B
build/nux/style.css 743 B
build/plugins/index.min.js 1.83 kB
build/primitives/index.min.js 921 B
build/priority-queue/index.min.js 582 B
build/react-i18n/index.min.js 671 B
build/redux-routine/index.min.js 2.63 kB
build/reusable-blocks/index.min.js 2.28 kB
build/reusable-blocks/style-rtl.css 256 B
build/reusable-blocks/style.css 256 B
build/rich-text/index.min.js 10.6 kB
build/server-side-render/index.min.js 1.32 kB
build/shortcode/index.min.js 1.48 kB
build/token-list/index.min.js 562 B
build/url/index.min.js 1.74 kB
build/viewport/index.min.js 1.02 kB
build/warning/index.min.js 248 B
build/widgets/index.min.js 7.27 kB
build/widgets/style-rtl.css 1.17 kB
build/widgets/style.css 1.18 kB
build/wordcount/index.min.js 1.04 kB

compressed-size-action

@paaljoachim
Copy link
Contributor

paaljoachim commented May 9, 2021

Thanks for working on this @creativecoder Grant!

I added the Post Title block to a page.

Before:
Screen Shot 2021-05-09 at 23 41 43

After:
Screen Shot 2021-05-10 at 00 11 15

Thank you for the additional controls! I went through and tested all of these. The controls works nicely!

As I added the block to a page. The page end up with having two titles. The static title and the Post Title block title.
Related issue that is being worked on these days. To moving the title to the top bar + having somekind of placeholder mini static title for the default title. #27093
Could also use additional feedback and likely developer help. Thanks.

@jasmussen
Copy link
Contributor

Cool, thanks for your work here. This is before:

Screenshot 2021-05-10 at 08 14 37

This is after:

Screenshot 2021-05-10 at 08 15 10

I know I've noted this in a couple of adjacent tickets, but it's important to reiterate. These text-decoration and letter case properties work well but are somewhat exotic/advanced controls, meaning until we have a better syste, for progressively adding these (as outlined in #27331), we should only be adding them to exotic/advanced blocks. In this case, adding it to Post Title seems fine, as that is exactly that: an advanced block as part of the FSE effort. So that's just to say, we need to refine the user experience before adding it to something like Paragraph.

@Addison-Stavlo
Copy link
Contributor

Addison-Stavlo commented May 10, 2021

Testing this out everything (except the below) seems to work well.

Im not sure if this is an issue with the hook used or specific to its implementation here, but when I try to press the "A" button in text decoration it doesn't seem to change anything:

Screen Shot 2021-05-10 at 10 54 41 AM

Also (more of a critique of this tool in general), there is no tooltip when hovering these buttons. I can't actually remember what "A" is supposed to do! 😆

@paaljoachim
Copy link
Contributor

@Addison-Stavlo
You are writing exactly the things that came to my mind which I forgot to write. :-)
As I too did not notice any reaction when clicking the "A" button. So when I wondered what it was I tried hovering the cursor on the "A" and expected a tooltip to show up, but there was none.

@jasmussen
Copy link
Contributor

I had to inspect the DOM to find the Aria label that says "None" to figure out what the A meant, so the icon isn't super clear as "none".

But digging a bit further, I'm now not sure where it's coming from. Looking at the Navigation block, which I believe is one of the first blocks to use the text-decoration control, it doesn't look like it has that "none" option:

Screenshot 2021-05-11 at 09 29 58

@aaronrobertshaw I think you worked on text decoration, do you recall where that's from?

@aaronrobertshaw
Copy link
Contributor

aaronrobertshaw commented May 11, 2021

@aaronrobertshaw I think you worked on text decoration, do you recall where that's from?

This isn't from the initial implementation of the text decoration block support. It was added as part of this PR we are looking at.

My guess is that the A was to represent normal text, no underline, no strikethrough. The character A was used to match the text transform support icons. That of course could just me be trying to belatedly interpret reasoning.

Originally, it was to use a ButtonGroup ( #26059 (comment) ) including an option for none for the user to be able to override something set by the theme. When it was decided it should use a segmented control, the none option wasn't included (#26060 (comment)).

@jasmussen
Copy link
Contributor

My guess is that the A was to represent normal text, no underline, no strikethrough. The character A was used to match the text transform support icons. That of course could just me be trying to belated interpret reasoning.

Good assessment, and I think it could work if it shared more DNA with the underline and strikethrough icons, say had the same font size and throughline.

Originally, it was to use a ButtonGroup ( #26059 (comment) ) including an option for none for the user to be able to override something set by the theme. When it was decided it should use a segmented control, the none option wasn't included (#26060 (comment)).

Thanks for the links, the segmented control still makes sense since for this property you can still only actually choose one at a time, not multiple. Thinking in terms of the user experience, the tricky bit here is to help indicate why the "none" option exists at all, when you can just untoggle one of the others. I.e. what's the difference between explicitly toggling "none" and not toggling any of the options? — I know the technical difference, but it's hard to convey.

We might just need better icons here and things will fall in place. I can look at this when I get a moment.

@aaronrobertshaw
Copy link
Contributor

I.e. what's the difference between explicitly toggling "none" and not toggling any of the options? — I know the technical difference, but it's hard to convey.

Exactly. The difference between an explicit none or "clear decoration" and the "default" or "inherited". It's a tricky one to convey via an icon.

@jameskoster
Copy link
Contributor

I like how Figma represents "none" with a dash:

Screenshot 2021-05-11 at 10 11 35

This makes the pattern consistent and easy to identify across different controls.

Using a letter like the "A" is always going to be somewhat ambiguous since it can easily be confused for "upper case" or "lower case". I know these aren't text decoration properties but the average user probably does not, and the Letter Case control may not always be present to guide them.

@creativecoder
Copy link
Contributor Author

Thank you for all the feedback and sorry for the confusion on the "none" setting and the icon, let me try to clarify:

I grabbed the textColor icon as a placeholder to represent text-decoration: none, because it looked (kind of) like "unstyled" text. But I 100% agree that doesn't have enough intrinsic meaning, we need something else. Will try the dash, (please let me know the best source of an SVG for that 🙂).

The option to use none here is important for overriding theme and/or browser styles. For example, Post Titles in the index template are always underlined in TT1 Blocks, but this isn't actually because of the theme, it's the browser's default style for links. Without the none option, there's no way to turn off the underline.

I'll proceed with the following:

  • Break out the addition of a "none" option for __experimentalTextDecoration into a separate PR as @nosolosw suggested--and maybe look into the labeling issue, while I'm at it.
  • In this PR, remove the css "hack" and try out the __experimentalSkipSerialization flag to target the text-decoration styles on the a tag. This should fix the issue of the None button seeming to do nothing, as @Addison-Stavlo and @paaljoachim pointed out.

@Addison-Stavlo
Copy link
Contributor

Now that I know that the regular "A" means "none"... this totally makes sense. 😆 🤦‍♀️

The option to use none here is important for overriding theme and/or browser styles.

Great point!

@carolinan
Copy link
Contributor

A text that is not a link should almost never be underlined, this is a big accessibility issue.
I do not think it is suitable to expose the underline control to users.

@alexstine
Copy link
Contributor

I'll second @carolinan . My belief is most text formatting can get users in to trouble. Not so much for screen readers as much as it would effect people with low vision who rely on those different looking text clues.

I think this whole PR could use a review from the Accessibility team as a whole and see if any other of these formatting concerns could come up aside from the underline.

Thanks.

@joedolson
Copy link
Contributor

Underlining unlinked text does not directly trigger an accessibility problem, but for sighted users underlining text that is not linked creates usability problems. Users will reflexively expect the underlined text to be a link. I wouldn't consider this an accessibility problem, per se, since it impacts all users who are able to perceive the underline pretty much equally.

Whether we should offer this as an option or not is somewhat a general question of whether WordPress should encourage best practices.

Underlined text that isn't linked is generally a poor practice from a design and usability standpoint. I generally think that WordPress should avoid offering this as an option.

Regarding other options:

Does the strikethrough toggle the semantic state of the text? If it wraps the text in <del> elements, that's fine; that will be communicated to most modern screen readers. Text decoration only will not, however, and I can't think of any cases where strikethrough is used purely as decoration for text.

All caps can also create accessibility problems, but since users have always been able to get around that by typing in all caps, I don't see any reason not to provide that option.

@creativecoder
Copy link
Contributor Author

Separate PR for adding the "none" option to the text decoration controls: #31768

@carolinan carolinan removed the Needs Accessibility Feedback Need input from accessibility label May 14, 2021
@alexstine
Copy link
Contributor

A summary of the Accessibility teams discussion can be found here.

https://make.wordpress.org/accessibility/2021/05/14/accessibility-team-meeting-notes-may-14-2021/

@ianstewart
Copy link
Contributor

It was decided that it would be better to remove the underline formatting option from the pull request for now and maybe revisit implementation later if there is a greater need.

It looks like removing the underline was the recommendation for a11y, is that correct?

@carolinan
Copy link
Contributor

carolinan commented Jun 9, 2021

The underline itself, when the post title is linked, should be the default text decoration.

A formatting option should not make it possible to add underlines to the text when it is not a link.

At some point in the future, I trust that there will be ways to handle text decorations and hover styles and colors for links. It is unclear if is a good idea to expose it as controls to users.

@jasmussen
Copy link
Contributor

jasmussen commented Jun 9, 2021

In my mind, exposing underline controls to users for links or headings make sense primarily in the sense that they would empower you to "fix" a bad theme that you otherwise love: maybe the theme comes with no underlines to those links, and by surfacing the tools you can add them back. You could also have done that by choosing another theme, or by writing custom CSS. But the underline tool itself is agnostic, it can be used for good or evil.

What we can do is push towards good defaults and practices. Using a progressive reveal system, we can hide underline controls by default, adding friction. We can even show contextual warnings or notices when you use the features in a bad way, e.g. if you employ an underline on a paragraph, we can output a big text warning — just like the contrast checker — explaining why that's a bad idea. That kind of education, paired with the tools, might ultimately be the better solution than hiding the controls.

@alaczek
Copy link

alaczek commented Jun 15, 2021

In my mind, exposing underline controls to users for links or headings make sense primarily in the sense that they would empower you to "fix" a bad theme

Yes, totally agree. This has been my experience so far. Without these controls, there's simply no way to get around theme default choices.

How could we move this PR forward? From my perspective, the controls that would make the biggest impact design-wise are font weight and font style. If text decoration and transformation need more discussion, maybe we could break them out into a separate PR, and get the other enhancements shipped?

@mtias
Copy link
Member

mtias commented Aug 27, 2021

Can we get all the other options except text-decoration merged and continue any approach there separately?

@glendaviesnz
Copy link
Contributor

@creativecoder are you wanting to continue with this PR or are you happy for someone else to make the change Matias suggests to get it over the line and merged?

@creativecoder creativecoder force-pushed the add/post-title--text-style-options branch from 4de6370 to 890ef1d Compare September 14, 2021 18:47
@creativecoder
Copy link
Contributor Author

@mtias @glendaviesnz Sorry for the delay, I lost track of this one. It's now updated and rebased, but omitting the text decoration controls.

A note about testing, there seems to be an issue with using the "Make a title a link" setting in the Post Title block, I get Uncaught TypeError: can't access property "rendered", fullTitle is undefined from PostTitleEdit edit.js:97 when trying to toggle it. (cc @ntsekouras , in case 7428d7e is related). But that's not related to this change, as I see the same problem on trunk, as well.

This should be ready for review... there's not much to it, now!

@apeatling apeatling self-requested a review September 14, 2021 20:52
Copy link
Contributor

@apeatling apeatling left a comment

Choose a reason for hiding this comment

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

Tested and confirmed all settings are working as expected. 👍

2021-09-14 13 59 39

@carolinan
Copy link
Contributor

Works well for me, and I also do not see a JS error for the link (Chrome, Windows 10).

@carolinan carolinan merged commit acccef8 into trunk Sep 15, 2021
@carolinan carolinan deleted the add/post-title--text-style-options branch September 15, 2021 11:57
@github-actions github-actions bot added this to the Gutenberg 11.6 milestone Sep 15, 2021
@creativecoder
Copy link
Contributor Author

I also do not see a JS error for the link (Chrome, Windows 10)

Good to know! Maybe it's just me.

fullofcaffeine added a commit that referenced this pull request Sep 16, 2021
* trunk: (74 commits)
  Update docs for ClipboardButton component (#34711)
  Post Title Block: add typography formatting options (#31623)
  Bump plugin version to 11.5.0
  Navigation Screen: Adjust header toolbar icon styles (#34833)
  Fix the parent menu item field in REST API responses (#34835)
  Rewrite FocusableIframe as hook API (#26753)
  Create Block: Remove wp-cli callout since not recommended and outdated (#34821)
  Global Styles: Fix dimensions panel default controls display (#34828)
  [RNMobile][Embed block] Enable embed preview for Instagram and Vimeo providers (#34563)
  Increase Link UI search results to 10 on Nav Editor screen (#34808)
  Prevent welcome guide overflow x scroll (#34713)
  Enable open on click for Page List inside Navigation. (#34675)
  [RNMobile] [Embed block] -  Unavailable preview fallback bottom sheet title update  (#34674)
  Add missing field _invalid in menu item REST API (#34670)
  Fix Dropdown/DropdownMenu toggle closing in all UAs (#31170)
  Navigation submenu block: replace global shortcut event handlers with local ones (#34812)
  Navigation Screen: Consolidate menu name and switcher (#34786)
  Remove parent and position validation from menu item REST API endpoint (#34672)
  Clean theme data when switching themes in the customizer (#34540)
  Components: add reset timeout to ColorPicker's copy functionality. (#34601)
  ...
@paaljoachim
Copy link
Contributor

As the Post Title Block has been taken care of it seems natural to help @vdwijngaert Koen with the PR on moving the Post/Page title to the top bar so that we can perhaps get it into WP 5.9.
The PR is here: #31288
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Post Title Affects the Post Title Block [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.