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

Don't use a details element for the metaboxes area #65406

Open
2 tasks done
afercia opened this issue Sep 17, 2024 · 4 comments · May be fixed by #65466
Open
2 tasks done

Don't use a details element for the metaboxes area #65406

afercia opened this issue Sep 17, 2024 · 4 comments · May be fixed by #65466
Assignees
Labels
[Feature] Meta Boxes A draggable box shown on the post editing screen [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Edit Post /packages/edit-post [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@afercia
Copy link
Contributor

afercia commented Sep 17, 2024

Description

Follow-up to #64351

Under a viewport height of 550 pixels, the metaboxes area container now turns into a <details> element. See

if ( isShort ) {
return (
<details
className={ className }
open={ isOpen }
onToggle={ ( { target } ) => {
setPreference(
'core/edit-post',
'metaBoxesMainIsOpen',
target.open
);
} }
>
<summary>{ __( 'Meta Boxes' ) }</summary>
{ contents }
</details>
);
}

As reported on #64351 (comment) by @joedolson a <details> element isn't ideal in this case. Basically it should not be used for sections of content that need a heading to identify the section. Also, it should not be used for accordion-like UIs. Rather, an expandable section with a heading 'Meta Boxe' should be used instead.

Step-by-step reproduction instructions

  • Activate a plugin that adds a metabox e.g. Yoast SEO.
  • Edit a post.
  • Emulate a viewport height that is smaller than 550 pixels.
  • Observe the metaboxes area turns into a <details> element.

Screenshots, screen recording, code snippet

metaboxes details

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). [Feature] Meta Boxes A draggable box shown on the post editing screen [Package] Edit Post /packages/edit-post labels Sep 17, 2024
@stokesman
Copy link
Contributor

Thank you for making the issue.

Rather, an expandable section with a heading 'Meta Boxe' should be used instead.

Is an additional heading a must-have? I'm asking because currently all meta box headings are h2 from core WP and I don’t believe it’s possible to change those to h3s (aside from runtime JS hacks). Perhaps it would be enough that the meta box headings are never accessibly hidden.

@afercia
Copy link
Contributor Author

afercia commented Sep 18, 2024

I understand changing on the fly the heading levels is a problem and probably not a good option.

However, the metabodes area must be findable and headings are one of hte most used navigational tools for screen readers. Any section of content should be identified by a heading.

@joedolson
Copy link
Contributor

An additional heading isn't a must-have; but if the metabox headings are not available, then there does need to be a heading to locate the section. The metabox heading level was set based on the assumption they would be visible; hiding them means that now a user needs to be able to locate the section they are in.

@stokesman stokesman linked a pull request Sep 18, 2024 that will close this issue
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Sep 18, 2024
@stokesman
Copy link
Contributor

stokesman commented Sep 18, 2024

#65466 tries an approach replacing the details element so that collapsing the meta boxes doesn’t remove them from the accessibility tree. However, maybe it’d be okay to hide them if the section itself is locatable. I'm opening to doing whatever’s best. Besides that I'm sure there are more details to consider. I’d appreciate any feedback over on that PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Meta Boxes A draggable box shown on the post editing screen [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Edit Post /packages/edit-post [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
Status: 🏗️ In Progress
Development

Successfully merging a pull request may close this issue.

3 participants