Remove unnecessary tabindex attributes from offcanvas docs examples

[anotherjames]

Description

Adjusts code examples in the offcanvas component's documentation in order to avoid potential bugs in Safari due to its unusual focus handling.

Motivation & Context

Having copied the code examples of the offcanvas component in an implementation for a website, I was surprised to find the presence of the tabindex attribute on the wrapping <div> caused unexpected behaviour in Safari. I had an event listener for the focusin event on an <a> element within the offcanvas component. When clicking that link element, Safari sets the event target to the <div>, unlike Chrome which sets it to the <a>. (Safari has a history of unusual focus handling!) This meant my event listener acted on the wrong element, causing a bug report from my client.

In an early commit of the original Offcanvas component's PR (https://github.com/twbs/bootstrap/pull/29017), the tabindex attribute on the wrapping <div> was being manually adjusted in javascript. But that no longer happens (e.g. since https://github.com/twbs/bootstrap/pull/33865 changed the way keyboard navigation & focus was handled anyway), yet the initial tabindex attribute remained in the code examples in its docs. Given that the wrapping <div> element wouldn't be tabbable without the tabindex attribute anyway, I believe the attribute is unnecessary. I suggest we remove it to save other developers who copy the examples from similar headaches!

Type of changes

• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Refactoring (non-breaking change)
• [ ] Breaking change (fix or feature that would change existing functionality)

Checklist

• [x] I have read the contributing guidelines
• [x] My code follows the code style of the project (using npm run lint)
• [x] My change introduces changes to the documentation
• [x] I have updated the documentation accordingly
• [ ] I have added tests to cover my changes
• [ ] All new and existing tests passed

Live previews

• https://deploy-preview-41524--twbs-bootstrap.netlify.app/

Related issues

[julien-deramond]

Thanks for reporting this issue and submitting a PR, @anotherjames! Looping in @patrickhlauke for accessibility feedback 🙏

Thinking out loud: since the offcanvas component behaves similarly to a modal in terms of focus management, if we move forward with this change, we might want to consider whether a similar adjustment should also apply to modals—assuming it doesn't negatively impact accessibility or focus behavior.

[patrickhlauke]

unfortunately, simply removing the tabindex and not programmatically moving focus at least to or into the offcanvas causes issues.

Video: using Chrome/NVDA, looking at the deploy preview. triggering one of the example offcanvases, we see focus still remains on the trigger button in the underlying page. using cursor up/down to read/navigate in NVDA, we're still going through the page behind the offcanvas. it's only if we hit TAB once the offcanvas is open that we end up inside the offcanvas and "trapped". for comparison, video includes the current behaviour, where focus is programmatically moved to the offcanvas.

https://github.com/user-attachments/assets/2da47b0b-3ae3-4c09-821f-2ea9bb223638

Now, moving focus to the offcanvas itself (as with modals) can indeed have unintended side effects - for instance, you may not want its entire contents to be read out in one big go. A better solution is to programmatically move focus to some element at the top/start of the offcanvas...

[anotherjames]

Interesting! Why is a tabindex=-1 on a div doing anything though? Div elements shouldn’t be tabbable anyway, so I would have thought this change was harmless (even if there are other improvements which could be made) ?

[patrickhlauke]

tabindex="-1" doesn't mean "tabbable", it means "programmatically focusable". otherwise, you call .focus() in JS but focus isn't really moved

[anotherjames]

tabindex="-1" doesn't mean "tabbable", it means "programmatically focusable". otherwise, you call .focus() in JS but focus isn't really moved

Huh, OK, I see! Well maybe there's an idea in here somewhere, but probably needs something with wider thought then. I just hope we all keep testing our sites in Safari whilst it continues to behave 'differently' due to the tabindex. So easy to miss! :)