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

Incorrect headings hierarchy in the Site Editor #42373

Closed
afercia opened this issue Jul 12, 2022 · 18 comments · Fixed by #51696
Closed

Incorrect headings hierarchy in the Site Editor #42373

afercia opened this issue Jul 12, 2022 · 18 comments · Fixed by #51696
Assignees
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Bug An existing feature does not function as intended

Comments

@afercia
Copy link
Contributor

afercia commented Jul 12, 2022

Description

In the Site Editor Template browser, the headings hierarchy is incorrect:

  • it starts with a H2
  • it skips heading levels

Step-by-step reproduction instructions

  • Hint: use the Chrome extension HeadingsMap to quickly check the headings on the page.
  • Go to the Site Editor.
  • Click the 'Show template details' button (the one with the chevron icon in the top bar).
  • Click 'Browse all templates'.
  • Check the headings on the page with the HeadingsMap extension.
  • Observe the first heading is the H3 'Editor' in the left sidebar.
  • Observe the second heading is the H1 'Templates' in the top bar: this is incorrect because the main H1 heading should be the first heading on a page.
  • Observe all the template listed on the page are H4 headings: this is incorrect because the H2 and H3 levels are skipped.

Screenshots, screen recording, code snippet

headings templates browser

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@afercia afercia added [Type] Bug An existing feature does not function as intended [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). labels Jul 12, 2022
@afercia
Copy link
Contributor Author

afercia commented Jul 12, 2022

Note: I think the Component that uses the H4 heading with the template name is reused in other places (the Show tempalte details panel, the Settings sidebar). I'm not sure it should use a heading in the first place, though.

@afercia afercia self-assigned this Jul 14, 2022
@afercia
Copy link
Contributor Author

afercia commented Jul 14, 2022

Updating the issue title as the problem applies to the whole site editor.

@afercia afercia changed the title Incorrect headings hierarchy in the Site Editor Template browser Incorrect headings hierarchy in the Site Editor Jul 14, 2022
@afercia
Copy link
Contributor Author

afercia commented Jul 15, 2022

The main H1 heading in the Site Editor is rendered by the DocumentActions component:

Screenshot 2022-07-15 at 15 33 44

There's some visually hidden text there. The actual heading text is, for example:

Editing template: Home

While the heading text is meaningful, the heading placement is not ideal. Actually, this H1 heading comes after the Navigation sidebar and after the toolbar in the header.

Ideally, the main H1 should be the first thing within the main content. This isn't just for better semantics. Worth reminding that headings are used also as navigational elements by screen reader users. They can jump through headings using dedicated keyboard shortcuts and by doing so there's a chance they could not be aware of (and thus skip) the content placed before the heading.

I think the main H1 should be generated on the WordPress PHP side, as it's the case for the H1 in the post editor. I'll create a core ticket on Trac.

@afercia
Copy link
Contributor Author

afercia commented Jul 15, 2022

Created https://core.trac.wordpress.org/ticket/56228 for the core part.

@joedolson
Copy link
Contributor

I'd really like to see this end up in WordPress 6.2; can we coordinate with people to get the PR merged? Not sure who to connect with; if @annezazu can suggest who should own this, that would be great. Proper headings are a basic level of accessibility that will really help users.

I can just commit https://core.trac.wordpress.org/ticket/56228, but by itself that will result in a duplicate H1.

@annezazu
Copy link
Contributor

Thanks for tagging me in this! I've added it to the 6.2 board for consideration. @youknowriad since this touches on Browse mode a bit, I wanted to lightly note for you in case you might have time to help with this.

@youknowriad
Copy link
Contributor

Thanks for creating the issue.

I think the main H1 should be generated on the WordPress PHP side, as it's the case for the H1 in the post editor. I'll create a core ticket on Trac.

I'm not sure this is good solution because the site editor is essentially an SPA meaning, there will be navigation that is supposed to change the H1.

The latest iteration of the site editor includes a "hub" that shows the current context. I think that's the ideal place to show the H1 (the currently edited template/template-part...). It's just a div now but it can be made an H1.

I'm also going to ping @WordPress/gutenberg-design here as they're working on small iterations there so we can include this fix in the iterations that are planned for 6.2

@afercia
Copy link
Contributor Author

afercia commented Jan 31, 2023

@youknowriad in the related PR #42495 the current H1 of this 'SPA' will stay functionally unchanged. For example the current H1 Editing template: Single item: Post will just change to a H2.

The core patch will add a visually hidden H1:
<h1 class="screen-reader-text hide-if-no-js"><?php _e( 'Edit site' ); ?></h1>

I think this is the most reasonable thing to do from a semantics and accessibility perspective.

  • The current H1 text Editing template: Single item: {template name} is arguably a text that communicates 'what' the page is about. It's more an indication of the current state.
  • The main H1 should clearly clarify where users landed and what the page is about. No different from all the other H1s in the WP admin pages e.g. 'Posts', 'Edit post', 'Categories', 'Edit category', 'Media Library', 'Edit Media', etc.

@youknowriad
Copy link
Contributor

The current H1 text Editing template: Single item: {template name} is arguably a text that communicates 'what' the page is about. It's more an indication of the current state.

I think it's arguable since that state is persisted in the URL and refreshing the page will land you directly in the "editing said template" state.

@alexstine
Copy link
Contributor

I actually have a different opinion, please see my comment here.

https://core.trac.wordpress.org/ticket/56228#comment:14

  1. There should be a heading 1 as it helps with context to let users know they are in the site editor.
  2. It should not be done in PHP as region navigation in React has not been initialized yet. This can lead to a weird experience until the hook is ready to use.

@afercia
Copy link
Contributor Author

afercia commented Feb 6, 2023

@alexstine thanks. I'm not sure I understand how region navigation relates to the main H1 heading. Can you please expand a bit, when you have a change?

It's important to take into consideration that the whole Editor renders within a page that is generated by core. There is a main landmark there, which is the element with ID wpbody:

<div id="wpbody" role="main">

That's equivalent to a <main> element. It's a good practice to place the main H1 heading as first thing within the main element.

Also, things are a bit more complex after the introduction of the Site editor 'browse more', which is the initial state of the Site editor. This makes the user experiences in the Site editor a it different from the one in the Post editor.

In the Site editor 'browse mode', right now there's only a H2 heading that says 'Design'. That's less than ideal and needs to be fixed.

Screenshot 2023-02-06 at 12 28 59

@alexstine
Copy link
Contributor

@afercia Try this.

  1. Create a new post.
  2. Try pressing ctrl+ or ctrl+shift+ to move forward/backward in Gutenberg region navigation.
  3. Notice how it does not work because focus has not gone through the React app yet. The heading is outside of the React app so the region hook cannot function.

Moving the heading 1 under React management should avoid this problem.

Thanks.

@afercia
Copy link
Contributor Author

afercia commented Feb 13, 2023

@alexstine thanks, for the clarification I understand now.
I'm not sure that depends on the presence of the H1 heading by the way.
It's just that initial focus may not be set anywhere. It's inconsistent across the Post Editor and the Site Editor and also depends on whether the Post Editor is in Fullscreen mode or not, or whether it's a new post or an existing one.

It's a bit complicated, pretty confusing.

  • New post: initial focus is set to Add Title. It's inside the editor so that the keyboard shortcuts work.
  • When editing an existing post, initial focus is not set anywhere:
    • With Fullscreen mode enabled, you need to tab at least once. At that point, you're inside the editor and the keyboard shortcuts work.
    • With Fullscreen mode disabled: the WP admin menu is shown so you need to tab several times to get to the editor, where the keyboard shortcuts will finally work.
    • In the Site Editor, initial focus is not set anywhere as well. The Site Editor is always in Fullscreen mode so you need to tab at least once, At that point, you're inside the editor and the keyboard shortcuts work.

Placing the H1 inside the editor would only facilitate a bit the flow, as in:

  • Jump to the H1 via screen reader shortcuts.
  • At that point, you're inside the editor and the keyboard shortcuts work.

This would only work for screen reader users. It won't work for other keyboard users, as they can't 'jump' to the H1.

However, there are countless ways for screen reader users to jump directly into the editor e.g. jumping to a H2 heading, jumping to the first form control, etc.

@alexstine
Copy link
Contributor

@afercia The h1 is still outside of the React part of Gutenberg though so it will not work. The flow would look like this for screen reader users.

  1. Find first heading 1.
  2. Press tab.
  3. Region navigation now works.

@ndiego ndiego moved this from 📥 Todo to 🏗️ In Progress in WordPress 6.2 Editor Tasks Feb 23, 2023
@annezazu
Copy link
Contributor

@SaxonF @jameskoster @jasmussen flagging this for you all as browse mode evolves for 6.2. In particular, it looks like #47241 was overwritten amidst the various changes (discovered when chatting with @carolinan) and it would be great to get it incorporated back in.

@jasmussen
Copy link
Contributor

Thanks for the ping. Definitely seems like a bug to fix. Does it need any design input? Apologies if I'm missing nuance, but this seems mostly like an oversight.

@joedolson
Copy link
Contributor

I don't think it needs any design feedback; the headings don't need to look any different from the existing text, they just need to be semantically fixed.

@ndiego ndiego moved this from 🏗️ In Progress to 🐛 Punted to 6.2.1 in WordPress 6.2 Editor Tasks Mar 9, 2023
@Mamaduka Mamaduka moved this from 🐛 Punted to 6.2.1 to 🦵 Punted to 6.3 in WordPress 6.2 Editor Tasks Apr 13, 2023
@ndiego ndiego moved this to ❓ Triage in WordPress 6.3.x Editor Tasks May 23, 2023
@ndiego ndiego moved this from ❓ Triage to 🏗️ In Progress in WordPress 6.3.x Editor Tasks May 23, 2023
@joedolson
Copy link
Contributor

Does this need any assistance to get it moving?

@github-project-automation github-project-automation bot moved this from 🏗️ In Progress to ✅ Done in WordPress 6.3.x Editor Tasks Jun 21, 2023
@priethor priethor removed the [Status] In Progress Tracking issues with work in progress label Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Bug An existing feature does not function as intended
Projects
No open projects
8 participants