Add heading hierarchy checker notice

[Jackie6]

Description

Fix #10581

How has this been tested?

• local test
• browser test

Screenshots

Types of changes

Add a checker to check if the selected heading level is correct

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.

[Jackie6]

@AmartyaU Can you explain further about this issue?

[AmartyaU]

@Jackie6 and I talked about the content of the error messages, which has also been debated about in the parent issue. We also have a solution that details the exact heading level to choose in the error message, something like "This heading level is incorrect. Please use H1, or H2 instead."

Our question is if the error message should be short but vague or long and detailed?

[afercia]

Thanks for working on this! Works pretty well :)

Stepping in after @gziolo invitation to give some feedback on the message wording. Personally, I'd keep it as is because in the sidebar the message should be short, simple, and informative. "This heading level is incorrect." seems OK to me.

However, I'm wondering if the message should also point users to the "Content structure" tool in the top bar because:

• the information there is more detailed
• it would help improve discoverability of the "Content structure" tool

There are cases "Content structure" definitely gives more context. For example, a H1 in the post content may be correct in some edge cases. Screenshots to illustrate the added context:

I know @joedolson has some thoughts on the message wording, I'd like to hear his opinion :)

[mapk]

I'm wondering if the message should also point users to the "Content structure" tool in the top bar

@afercia how do you imagine this working? Would there be a link that opened this popover from the Block Inspector in relation to the messaging?

[Jackie6]

Hi, @afercia Thanks for comments. I could not imagine the edge cases where the H1 heading is correct, could you illustrate this a bit more? And I have the same question as @mapk, how would you expect to open the popover?

[joedolson]

The edge cases where an H1 would be correct would be pretty obscure, I think. I can think of at least one - if the theme omits the page title and the heading is the first block of content in the post. While that's unusual, I have seen cases, probably due to people wanting a different title used in lists of links than displayed on the page.

I agree with the short wording; I think attempting to elaborate the wording in this context isn't beneficial. Giving a brief warning then providing access to the more detailed information would seem more user-friendly.

[afercia]

The edge cases where an H1 would be correct would be pretty obscure, I think. I can think of at least one - if the theme omits the page title

Yep, also some post formats in some themes don't have a h1 heading, e.g. Twenty Thirteen "status" (if I remember correctly). Also, custom post types might not have a post title as well.

[afercia]

how do you imagine this working? Would there be a link that opened this popover from the Block Inspector in relation to the messaging?

Wasn't actually thinking at a control to open the Content Structure. Interaction would be tricky: focus should be moved there, then moved back to the invoking context when closing, etc.

I was just thinking to specify in the text something like:
"Hey dear user, this heading level is not correct. You may want to check the Content Structure tool for more context". With some shorter wording, of course :)

[gziolo]

Nice work, I left some feedback. It's mostly about code-related improvements. The functionality itself seems to work as expected but that you can polish with accesibility experts :)

[gziolo]

@jorgefilipecosta - is there a way to get all heading blocks only so we didn't have to use computeOutlineHeadings in the first place?

There is getGlobalBlockCount, so something similar would be nice.

That way we could share the same selector in both places and remove this computeOutlineHeadings helper altogether.

[Jackie6]

In fact, headings has been computed in

gutenberg/packages/editor/src/components/document-outline/index.js

Lines 67 to 69 in 8722f49

	export const DocumentOutline = ( { blocks = [], title, onSelect, isTitleSupported, hasOutlineItemsDisabled } ) => {
	const headings = computeOutlineHeadings( blocks );

I am not sure whether we could avoid computing it again.

[gziolo]

We can if we create memoized shared selector, similar to how getBlocks is cached as of today. It might be a complicated task and that's why I asked @jorgefilipecosta for help.

[jorgefilipecosta]

We could create a generic selector that returns the blocks of a given type, so it is not specific to this functionality.
But I'm not sure if it would fit our needs, computeOutlineHeadings does more than just returning the heading blocks, it computes, for example, the heading path required to show the path where a heading is nested on.
I think in this specific case just the selector would work, but in the checks, we do in the Content Structure we would still need something like computeOutlineHeadings to associate a path to each heading.
Exposing a selector that does what computeOutlineHeadings does (or similar functionality with paths) seems very specific to this functionality.
I guess a possible path here is just implementing level-checker.js as part of the editor module and reusing computeOutlineHeadings there.

[AmartyaU]

I was just thinking to specify in the text something like:
"Hey dear user, this heading level is not correct. You may want to check the Content Structure tool for more context". With some shorter wording, of course :)

@afercia I love this solution. It would be the most user-friendly without complicated code and recurring feedback.

Something like "This heading level is incorrect. Check the Content Structure tool for more context."

We can also link 'Content Structure tool' to a resource explaining what the tool is for people who have never used it before? Like you said, it would also increase discoverability this way!

[afercia]

We can also link 'Content Structure tool' to a resource explaining what the tool is for people who have never used it before. Like you said, it would also increase discoverability this way!

Yep, good point. Worth noting Gutenberg and core will do this starting from 5.2 for the alt text description 🙂linking to the W3C alt text tutorial

We'd just need to identify a good resource (better if translated or translatable). I guess a link could also be added in a separate PR.

[ZebulanStanphill]

It looks like this PR has fallen stale/inactive. I've picked up the work in #22650. Thank you for getting this train moving, @Jackie6! It would have been a lot harder to get this feature working without your initial effort!