Skip to content

Commit

Permalink
hotfix: focus-visibile trigger false positives on mutated active element
Browse files Browse the repository at this point in the history
TL;DR for a brief moment focus-visible plugin sets document.activeElement to body(proably due to the blur() event on their codebase). that trigger our code to "think" that the active element was mutated — when hidden/removed focus moves to body. So we though that our tabbed element was deleted initiation that spaghetti of finding next, but since nothing was removed, focus visible set "next" to body, so our modal started from fist focus element again. Result we were always trapped in the same element. Solution is to wrap verification in an empty setTimout, that moves our code to the next queue, and in that place focus-visible has done its thing and active element is back to normal.

This reveals the fragility of my focusTrap code. I know that is overengineered, but looking at other dialog/modal implementations it seems to me that either i'm not seeing something obvious, or nobody ecounters edge cases as frequent as me.

closes #23. But #22 is desperately need 🆘
  • Loading branch information
renatodeleao committed Jun 9, 2020
1 parent 9b45d54 commit 214225b
Showing 1 changed file with 32 additions and 15 deletions.
47 changes: 32 additions & 15 deletions src/A11yVueDialogRenderless.vue
Original file line number Diff line number Diff line change
Expand Up @@ -415,23 +415,40 @@ export default {
// v-if might have happend, so listen for tick
this.$nextTick(() => {
this.getFocusableChildren();
// a lot of mutations can be triggerd because subtree, we just want the
/**
* Bugfix #23 - other plugins/code can attach focus/blur events to the
* elements that we're watching, triggering attribute mutatations. blur()
* event is particularly interesting because it sets document.activeElement
* to body — which on or dialog plugin brain means a false positive for
* hide/remove mutation of the current tabbed item (remmeber if activeElement
* is body, button was removed or hidden, as in not focusable.)
*
* focus-visible triggers blur() setting document.activeElement to body
* for a brief moment and then back to the element again. Moving our
* verification to the next queue with setTimeout workaround it.
*
* @see https://allyjs.io/tutorials/mutating-active-element.html
*/
setTimeout(() => {
// a lot of mutations can be triggerd because subtree, we just want the
// state at the last one. Nope it's not the same as putting outside of
// state at the last one. Nope it's not the same as putting outside of
// the loop. trust me, i've tried that.
if (i === mutationsList.length - 1) {
if (document.activeElement === this.focusable[this.focusedIndex]) {
return;
}
// [1]
if (document.activeElement === document.body) {
this.focusableMutated = true;
// focus on root to keep keyboard bindings working
this.dialogRoot.focus();
// state at the last one. Nope it's not the same as putting outside of
// the loop. trust me, i've tried that.
if (i === mutationsList.length - 1) {
if (document.activeElement === this.focusable[this.focusedIndex]) {
return;
}
// [1]
if (document.activeElement === document.body) {
this.focusableMutated = true;
// focus on root to keep keyboard bindings working
this.dialogRoot.focus();
}
}
}
})
});
}
}
Expand Down

0 comments on commit 214225b

Please sign in to comment.