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

Add semantic section elements #3432

Merged
merged 1 commit into from Apr 5, 2021
Merged

Conversation

selfthinker
Copy link
Collaborator

@selfthinker selfthinker commented Mar 6, 2021

This changes the divs for the main content, header, footer, sidebar and page tools to their semantic equivalent (main, header, footer, nav and nav again) and adds ARIA labels.
The media manager popup changes to have a main and a nav.

This also changes the divs around user tools, site tools and breadcrumbs to navs.
Because it is best practice to not have too many navigation landmarks but only the most important ones for screen reader users, this intentionally removes these navs' semantics again via role=presentation. Those are relatively easy to get to anyway as they are all in the header.
That makes it sort of unnecessary to add the navs in the first place, but I thought it's more consistent that way. (I'd also be happy to remove the second commit which does that.)

When using the Landmarks browser extension, the landmarks will become visible.


One question: I made the sidebar have a nav, but we cannot be sure if it is used as a navigation. If it is not used as one, an aside would be more appropriate. In the Starter template I solved this by making it configurable.
But I suspect you'd probably not want another config option in the core?
It would be good if we had more data on how the sidebar is used to make a better informed decision.

@selfthinker
Copy link
Collaborator Author

Hmm, the unit tests complain that role="presentation" is "invalid HTML". It is definitely not invalid HTML, though.
I see that it's a problem with the validator. I will file a bug report there.
Is there another way to fix the test?

@selfthinker
Copy link
Collaborator Author

I have reported the validator bug here: validator/validator#1104

@selfthinker
Copy link
Collaborator Author

selfthinker commented Mar 7, 2021

Well, <nav role="presentation"> was valid HTML yesterday but is invalid today as the spec was changed due to my bug report. (Although, apparently it was only unintentionally valid before.)
I will just remove the second commit which uses it.

This changes the divs for the main content, header, footer,
sidebar and page tools to their semantic equivalent
(main, header, footer, nav and nav again)
and adds ARIA labels where appropriate.
The media manager popup changes to have a main and a nav.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants