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

Media & Text: Remove font size declaration from template #40329

Merged

Conversation

roseg43
Copy link
Contributor

@roseg43 roseg43 commented Apr 13, 2022

What?

The Media & Text block includes a Paragraph block in its template that has a default font size of large. This is both an accessibility issue, and creates frustrations for both editors and developers.

Why?

WCAG 2.1 (Level A) Success Criterion 1.3.1 Info and Relationships states that “[…] Information and relationships that are implied by visual or auditory formatting are preserved when the presentation format changes”. By making the paragraph a larger font size, the block is effectively meeting the requirements of Failure F2 by emphasizing text without using semantic markup to convey the visual significance of the text to users of assistive technology.

Additionally, if a developer has removed custom font sizes from core/paragraph or the theme itself using theme.json, this class gets applied and then is not removable.

This addresses #21126

How?

This simply removes the font size declaration in the Media & Text block's template.

Testing Instructions

  1. Open a Post or a Page
  2. Insert a Media & Text block
  3. Add some text content to override the placeholder content
  4. Confirm that the first paragraph in the block no longer has the .has-large-font-size class added by default.

This represents and accessibility improvement to this block. WCAG 2.1 (Level A) Success Criterion 1.3.1 Info and Relationships states that “[…] nformation and relationships that are implied by visual or auditory formatting are preserved when the presentation format changes”. By making the paragraph a larger font size, the block is effectively meeting the requirements of Failure F2 by emphasizing text without using semantic markup to convey the visual significance of the text to users of assistive technology.

Additionally, if a developer has removed custom font sizes from the Paragraph block, or the site entirely via theme.json, the custom font size is still applied, with no way to remove it.
@roseg43 roseg43 requested a review from ajitbohra as a code owner April 13, 2022 21:39
@github-actions
Copy link

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @roseg43! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Apr 13, 2022
@roseg43
Copy link
Contributor Author

roseg43 commented Apr 13, 2022

An alternative to this solution if the intent is for the template content to act as a heading is to switch the template block over to core/heading. However, this represents an actual semantic change to the default block contents, which may result in theming inconsistencies with Media & Text blocks on older posts/pages.

@Mamaduka Mamaduka added [Type] Enhancement A suggestion for improvement. [Block] Media & Text Affects the Media & Text Block labels Apr 14, 2022
@carolinan
Copy link
Contributor

Regarding the second suggestion: The paragraph was selected for this block because it was not possible to know which heading level would be the correct one.

@github-actions
Copy link

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @roseg43! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@Mamaduka
Copy link
Member

Mamaduka commented Apr 14, 2022

Additionally, if a developer has removed custom font sizes from core/paragraph or the theme itself using theme.json, this class gets applied and then is not removable.

A similar issue has recently been fixed for the Cover block - #34993.

Personally, I'm okay with this change. But it would be nice to get more feedback. Cc @jameskoster, @jasmussen.

@jasmussen
Copy link
Contributor

My take: yes, okay to remove. Many of these older blocks came with somewhat more opinionated defaults in order to display their variety. As design tools and theme.json properties mature, most of these should be absorbed there, and could potentially evolve to style variant presets if need be. (Such presets would not be class-based as they are today, but rather, shortcuts to predefined design tool settings, such as adding a value to the border radius control rather than attaching that radius to CSS)

@roseg43
Copy link
Contributor Author

roseg43 commented Apr 14, 2022

Regarding the second suggestion: The paragraph was selected for this block because it was not possible to know which heading level would be the correct one.

That makes a lot of sense, thank you for the context behind the original decision! Given that context I think removing the font size declaration makes the most sense because ideally to meet the requirements of Success Criterion 1.3.1, paragraph text should look like paragraph text in its default state.

And I'll just add: While the larger size does show off the variety of the block, it's also inherently confusing to folks who aren't aware of the difference between semantic headings and text that looks like a heading. As an authoring tool, I would argue that Gutenberg should allow people to create less accessible content, but it should make it a deliberate choice when possible. To that effect, Media & Text is currently making it an unconscious decision.

@Mamaduka Mamaduka merged commit f2f7ea0 into WordPress:trunk Apr 14, 2022
@github-actions
Copy link

Congratulations on your first merged pull request, @roseg43! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts:

https://profiles.wordpress.org/me/profile/edit/

And if you don't have a WordPress.org account, you can create one on this page:

https://login.wordpress.org/register

Kudos!

@github-actions github-actions bot added this to the Gutenberg 13.1 milestone Apr 14, 2022
@Mamaduka
Copy link
Member

Thanks for the contribution, @roseg43! Congrats on your first merged PR in the Gutenberg project.

@roseg43
Copy link
Contributor Author

roseg43 commented Apr 14, 2022

Thanks @Mamaduka for reviewing this so quickly! Happy to contribute, and looking forward to contributing more in the future.

@fabiankaegy
Copy link
Member

I'm changing the label of this from Enhancement to Bug for two reasons:

  1. It is an accessibility issue with the semantic structure of the content
  2. If a developer has removed custom font sizes from core/paragraph or the theme itself using theme.json, the has-large-font-size class still gets applied and then is not removable via the UI.

@fabiankaegy fabiankaegy added [Type] Bug An existing feature does not function as intended and removed [Type] Enhancement A suggestion for improvement. labels Jun 16, 2022
@fabiankaegy
Copy link
Member

Hey @adamziel @peterwilsoncc 👋

I would like to raise this patch for consideration of having it included in a minor WordPress release. I've outlined my rationale for marking this as a bug in the comment above and would love to get your thoughts on it.

Thanks in advance :)

@adamziel adamziel added the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Jun 16, 2022
@adamziel
Copy link
Contributor

adamziel commented Jun 16, 2022

@fabiankaegy Thanks! I've added the backport label to make this come up during the triage.

adamziel pushed a commit that referenced this pull request Jun 21, 2022
@adamziel
Copy link
Contributor

I just cherry-picked this PR to the wp/6.0 branch to get it included in the next release: c032340

@adamziel adamziel removed the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Media & Text Affects the Media & Text Block First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants