Skip to content

Commit 3ecf67e

Browse files
andreslucenaandreslucena
authored
Fix focus guard related bugs (#16213)
1 parent 7c9cd5f commit 3ecf67e

8 files changed

Lines changed: 119 additions & 9 deletions

File tree

‎decidim-core/app/cells/decidim/report_button/already_reported_modal.erb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
<%= decidim_modal id: modal_id, class: "flag-modal" do %>
22
<div data-dialog-container>
33
<%= icon "flag-line" %>
4-
<h2 tabindex="-1" data-dialog-title><%= t("decidim.shared.flag_modal.title") %></h2>
4+
<h2 id="dialog-title-<%= modal_id %>" tabindex="-1" data-dialog-title><%= t("decidim.shared.flag_modal.title") %></h2>
55
<div>
66
<div class="form__wrapper flag-modal__form">
77
<p class="flag-modal__form-description"><%= t("decidim.shared.flag_modal.already_reported") %></p>

‎decidim-core/app/cells/decidim/report_button/flag_modal.erb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
<%= decidim_form_for report_form, builder:, url: report_path, method: :post, html: { id: nil, data: { controller: "report-form" } } do |f| %>
33
<div data-dialog-container>
44
<%= icon "flag-line" %>
5-
<h2 tabindex="-1" data-dialog-title><%= t("decidim.shared.flag_modal.title") %></h2>
5+
<h2 id="dialog-title-<%= modal_id %>" tabindex="-1" data-dialog-title><%= t("decidim.shared.flag_modal.title") %></h2>
66
<div>
77
<div class="form__wrapper flag-modal__form">
88
<p id="dialog-desc-<%= modal_id %>" class="flag-modal__form-description"><%= t("decidim.shared.flag_modal.description") %></p>

‎decidim-core/app/cells/decidim/report_user_button/already_reported_modal.erb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
<%= decidim_modal id: modal_id, class: "flag-user-modal" do %>
22
<div data-dialog-container>
33
<%= icon "flag-line" %>
4-
<h2 tabindex="-1" data-dialog-title><%= t("decidim.shared.flag_user_modal.title") %></h2>
4+
<h2 id="dialog-title-<%= modal_id %>" tabindex="-1" data-dialog-title><%= t("decidim.shared.flag_user_modal.title") %></h2>
55
<div>
66
<div class="form__wrapper flag-modal__form">
77
<p class="flag-modal__form-description"><%= t("decidim.shared.flag_user_modal.already_reported") %></p>

‎decidim-core/app/cells/decidim/report_user_button/flag_modal.erb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
<%= decidim_form_for report_form, builder:, url: report_path, method: :post, html: { id: nil, data: { controller: "report-form" } } do |f| %>
33
<div data-dialog-container>
44
<%= icon "flag-line" %>
5-
<h2 tabindex="-1" data-dialog-title><%= t("decidim.shared.flag_user_modal.title") %></h2>
5+
<h2 id="dialog-title-<%= modal_id %>" tabindex="-1" data-dialog-title><%= t("decidim.shared.flag_user_modal.title") %></h2>
66
<div>
77
<div class="form__wrapper flag-modal__form">
88
<p class="flag-modal__form-description"><%= t("decidim.shared.flag_user_modal.description") %></p>

‎decidim-core/app/cells/decidim/share_text_widget/modal.erb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<%= decidim_modal id: "socialShare", class: "share-modal" do %>
22
<div data-dialog-container>
3-
<h2 tabindex="-1" data-dialog-title><%= t("share", scope: "decidim.shared.share_modal") %></h2>
3+
<h2 id="dialog-title-socialShare" tabindex="-1" data-dialog-title><%= t("share", scope: "decidim.shared.share_modal") %></h2>
44

55
<div>
66

‎decidim-core/app/packs/src/decidim/a11y.js‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@ import Dialogs from "a11y-dialog-component";
88
* @return {void}
99
*/
1010
const createDialog = (component) => {
11+
const getFocusableElements = (container) => {
12+
const selectors = "a[href],button:not([disabled]),input:not([disabled]),select:not([disabled]),textarea:not([disabled]),[tabindex]:not([tabindex='-1'])";
13+
return Array.from(container.querySelectorAll(selectors)).filter(
14+
(el) => el.offsetParent !== null
15+
);
16+
};
17+
1118
const {
1219
dataset: { dialog, ...attrs }
1320
} = component;
@@ -29,11 +36,33 @@ const createDialog = (component) => {
2936
backdropSelector: `[data-dialog="${dialog}"]`,
3037
enableAutoFocus: false,
3138
onOpen: (params, trigger) => {
39+
const keyHandler = (event) => {
40+
if (event.key !== "Tab") {
41+
return;
42+
}
43+
const focusable = getFocusableElements(params);
44+
if (focusable.length === 0) {
45+
return;
46+
}
47+
if (event.shiftKey && document.activeElement === focusable[0]) {
48+
event.preventDefault();
49+
focusable[focusable.length - 1].focus({ preventScroll: true });
50+
} else if (!event.shiftKey && document.activeElement === focusable[focusable.length - 1]) {
51+
event.preventDefault();
52+
focusable[0].focus({ preventScroll: true });
53+
}
54+
};
55+
params._focusTrapHandler = keyHandler;
56+
params.addEventListener("keydown", keyHandler);
3257
setFocusOnTitle(params);
3358
window.focusGuard.trap(params, trigger);
3459
params.dispatchEvent(new CustomEvent("open.dialog"));
3560
},
3661
onClose: (params) => {
62+
if (params._focusTrapHandler) {
63+
params.removeEventListener("keydown", params._focusTrapHandler);
64+
Reflect.deleteProperty(params, "_focusTrapHandler");
65+
}
3766
window.focusGuard.disable();
3867
params.dispatchEvent(new CustomEvent("close.dialog"));
3968
},

‎decidim-core/app/packs/src/decidim/a11y.test.js‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
/* global jest */
2+
3+
import { createDialog } from "src/decidim/a11y"
4+
5+
describe("a11y dialog focus trap", () => {
6+
const dialogHtml = `
7+
<button id="open-btn" data-dialog-open="testDialog">Open</button>
8+
<div id="test-dialog" data-dialog="testDialog">
9+
<div data-dialog-container>
10+
<h2 id="dialog-title-testDialog" tabindex="-1" data-dialog-title>Test Dialog</h2>
11+
<a href="#">Link 1</a>
12+
<button>Button 1</button>
13+
<input type="text" />
14+
<button>Button 2</button>
15+
<a href="#">Link 2</a>
16+
</div>
17+
<div data-dialog-actions>
18+
<button data-dialog-close="testDialog">Close</button>
19+
</div>
20+
</div>
21+
`;
22+
23+
beforeEach(() => {
24+
document.body.innerHTML = dialogHtml;
25+
window.Decidim = {
26+
currentDialogs: {}
27+
};
28+
window.focusGuard = {
29+
trap: jest.fn(),
30+
disable: jest.fn()
31+
};
32+
});
33+
34+
describe("keydown handler", () => {
35+
let dialogEl = null;
36+
37+
beforeEach(() => {
38+
const component = document.querySelector("[data-dialog]");
39+
createDialog(component);
40+
dialogEl = document.querySelector("[data-dialog='testDialog']");
41+
// Get the dialog from Decidim.currentDialogs and open it
42+
const dialog = window.Decidim.currentDialogs.testDialog;
43+
dialog.open();
44+
});
45+
46+
it("adds keydown handler on open", () => {
47+
expect(dialogEl._focusTrapHandler).toBeDefined();
48+
});
49+
50+
it("handles Tab key", () => {
51+
const selectors = "a[href],button:not([disabled]),input:not([disabled]),select:not([disabled]),textarea:not([disabled]),[tabindex]:not([tabindex='-1'])";
52+
const tabbableElements = Array.from(dialogEl.querySelectorAll(selectors)).filter(
53+
(el) => el.offsetParent || el.offsetParent === null
54+
);
55+
tabbableElements[tabbableElements.length - 1].focus();
56+
57+
const event = new KeyboardEvent("keydown", { key: "Tab", bubbles: true });
58+
const preventDefault = jest.fn();
59+
event.preventDefault = preventDefault;
60+
61+
dialogEl.dispatchEvent(event);
62+
63+
expect(preventDefault).toHaveBeenCalled();
64+
});
65+
});
66+
67+
describe("onClose cleanup", () => {
68+
it("removes the keydown handler on close", () => {
69+
const component = document.querySelector("[data-dialog]");
70+
createDialog(component);
71+
const dialogEl = document.querySelector("[data-dialog='testDialog']");
72+
73+
const dialog = window.Decidim.currentDialogs.testDialog;
74+
dialog.open();
75+
expect(dialogEl._focusTrapHandler).toBeDefined();
76+
77+
dialog.close();
78+
expect(dialogEl._focusTrapHandler).toBeUndefined();
79+
});
80+
});
81+
});

‎decidim-core/app/packs/src/decidim/refactor/moved/focus_guard.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,16 +78,16 @@ export default class FocusGuard {
7878

7979
let target = null;
8080
if (guard.dataset.position === "start") {
81-
// Focus at the start guard, so focus the first focusable element after that
82-
for (let ind = 0; ind < visibleNodes.length; ind += 1) {
81+
// Focus at the start guard, so focus the last focusable element (cycle forward to end)
82+
for (let ind = visibleNodes.length - 1; ind >= 0; ind -= 1) {
8383
if (!this.isFocusGuard(visibleNodes[ind]) && this.isFocusable(visibleNodes[ind])) {
8484
target = visibleNodes[ind];
8585
break;
8686
}
8787
}
8888
} else {
89-
// Focus at the end guard, so focus the first focusable element after that
90-
for (let ind = visibleNodes.length - 1; ind >= 0; ind -= 1) {
89+
// Focus at the end guard, so focus the first focusable element (cycle back to start)
90+
for (let ind = 0; ind < visibleNodes.length; ind += 1) {
9191
if (!this.isFocusGuard(visibleNodes[ind]) && this.isFocusable(visibleNodes[ind])) {
9292
target = visibleNodes[ind];
9393
break;

0 commit comments

Comments
 (0)