Skip to content

Conversation

@nstrayer
Copy link
Contributor

This is a minimum fix for the accessibility of the sidebar title.

By using role="heading" we are effectively making the title-containing div look like an h2 tag (see implicit aria-level).

I think it may be better to use an h2 tag as we get predictable sizing and also more semantic HTML. While, in theory, a page could have a lot of nested sidebars, and this would be confusing, screen-readers seem to handle it fine.

Fixes issue where the current card header is not accessible due to the title looking like any other element.
@nstrayer
Copy link
Contributor Author

@schloerke

Copy link
Collaborator

@cpsievert cpsievert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! BTW, another unfortunate reason why we tend to avoid semantic headers is because stuff like {pkgdown} will try to include them in a table of contents

@cpsievert cpsievert requested a review from gadenbuie May 10, 2023 22:17
@gadenbuie
Copy link
Member

I've done some reading and research and I'm not sure we should add this role by default. If we do add it, then role="heading" is a better default choice than using <h2>. Functionally, they're equivalent but adding only the role will avoid TOC complications and having to do a lot of work to reset h2 or hN styles (in short: we'd take on a lot of complexity in our CSS rules to make things work out as expected in most situations).

I agree that using an appropriate heading, e.g. h2() or h3() is a good choice in many cases, and I think we could call that out better in the docs.

But the challenge we face in bslib with components like these is that we don't know where in the document the layout will appear or what contents it will contain. The specific context of the final usage has a big impact on the most appropriate markup. Our goal with our defaults is to do the best least bad thing we can that's reasonable in as many uses as possible, while leaving a viable path forward for better context-specific choices.

I do have a solution: to use a <header> element. We use role="complementary" on the sidebar div, which is equivalent to <aside> (and we should probably switch to using that element directly if we can). Within the aside, we can wrap the sidebar title in <header>, which will cause it to be announced appropriately as a banner even if the title element doesn't have a heading role. In the case when an author knows they should use an appropriately-leveled hX tag, e.g. h3(), they can still give the title argument that tag and the resulting markup is still a good choice – <header><h3>Sidebar Title</h3></header>.

@gadenbuie
Copy link
Member

I've opened a new issue to collection some thoughts and research before we move forward with a PR.

@gadenbuie gadenbuie closed this May 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants