Bugzilla

(In reply to :Gijs (he/him) from comment #7)
> Dan, I'm pretty confused by the [patch you landed](https://phabricator.services.mozilla.com/D157398) - you mention you were concerned about  getting tabIndex=-1 in "all the right places" - but it's on a bunch of `<h1>` tags which should never be focusable in the first place (so should be a no-op, I think?) and on the `<named-deck>`, where I'm not sure what exactly it's accomplishing. Certainly that last one is now breaking expected keyboard functionality. What breaks for the feature callout work if we get rid of it?

This was original motivated by this bit of bug 1790651 comment 0:

> But if the user wandered around, the best place to return the focus would be the last focusable element before the onboarding popup was injected in the DOM or on the topmost element right after it, i.e. when the last popup is dismissed, the focus could be placed on the aside section for Colorways, for instance with the tabindex="-1" on the Independent Voices heading, so the user does not miss any piece of the content after the dismissal"

Re-reading that, however, makes it clear that the tabindex stuff would only make sense if the "on the topmost element right after it" implementation had been chosen.

Since the patch used the "return focus to where it was" implementation, I suspect the right fix is simply to back out the tabindex changes.

Looking at the details of this a bit more, then will write an integration test and a patch to go with this afternoon.

(In reply to :Gijs (he/him) from comment #7)
> Dan, I'm pretty confused by the [patch you landed](https://phabricator.services.mozilla.com/D157398) - you mention you were concerned about  getting tabIndex=-1 in "all the right places" - but it's on a bunch of `<h1>` tags which should never be focusable in the first place (so should be a no-op, I think?) and on the `<named-deck>`, where I'm not sure what exactly it's accomplishing. Certainly that last one is now breaking expected keyboard functionality. What breaks for the feature callout work if we get rid of it?

This was originally motivated by this bit of bug 1790651 comment 0:

> But if the user wandered around, the best place to return the focus would be the last focusable element before the onboarding popup was injected in the DOM or on the topmost element right after it, i.e. when the last popup is dismissed, the focus could be placed on the aside section for Colorways, for instance with the tabindex="-1" on the Independent Voices heading, so the user does not miss any piece of the content after the dismissal"

Re-reading that, however, makes it clear that the tabindex stuff would only make sense if the "on the topmost element right after it" implementation had been chosen.

Since the patch used the "return focus to where it was" implementation, I suspect the right fix is simply to back out the tabindex changes.

Looking at the details of this a bit more, then will write an integration test and a patch to go with this afternoon.

(In reply to :Gijs (he/him) from comment #7)
> Dan, I'm pretty confused by the [patch you landed](https://phabricator.services.mozilla.com/D157398) - you mention you were concerned about  getting tabIndex=-1 in "all the right places" - but it's on a bunch of `<h1>` tags which should never be focusable in the first place (so should be a no-op, I think?) and on the `<named-deck>`, where I'm not sure what exactly it's accomplishing. Certainly that last one is now breaking expected keyboard functionality. What breaks for the feature callout work if we get rid of it?

This was originally motivated by this bit of bug 1790651 comment 0:

> But if the user wandered around, the best place to return the focus would be the last focusable element before the onboarding popup was injected in the DOM or on the topmost element right after it, i.e. when the last popup is dismissed, the focus could be placed on the aside section for Colorways, for instance with the tabindex="-1" on the Independent Voices heading, so the user does not miss any piece of the content after the dismissal"

Re-reading that, however, makes it clear that the tabindex stuff would only make sense if the "on the topmost element right after it" implementation had been chosen.

Since the patch used the "return focus to where it was" implementation, I suspect the right fix is simply to back out the tabindex changes.

Looking at the details of this a bit more, then will write an integration test and an implementation patch this afternoon (i.e. now).