Skip to content

Commit 5431c04

Browse files
Fix keyboard focus for location dropdown when opened via keyboard
Agent-Logs-Url: https://github.com/OneNoteDev/WebClipper/sessions/a4bc18dd-5306-49f5-accf-24b9644af25c Co-authored-by: KethanaReddy7 <257986085+KethanaReddy7@users.noreply.github.com>
1 parent b57856e commit 5431c04

2 files changed

Lines changed: 119 additions & 0 deletions

File tree

src/scripts/clipperUI/components/sectionPicker.tsx

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,27 @@ export class SectionPickerClass extends ComponentBase<SectionPickerState, Sectio
5656
Clipper.logger.logClickEvent(Log.Click.Label.sectionPickerLocationContainer);
5757
}
5858
this.props.onPopupToggle(shouldNowBeOpen);
59+
if (shouldNowBeOpen) {
60+
// After the popup renders, move keyboard focus to the currently selected section item
61+
// so that keyboard users can identify which item is selected and navigate from there
62+
setTimeout(() => {
63+
let curSectionId = this.state.curSection && this.state.curSection.section ? this.state.curSection.section.id : undefined;
64+
let elementToFocus: HTMLElement;
65+
if (curSectionId) {
66+
elementToFocus = document.getElementById(curSectionId) as HTMLElement;
67+
}
68+
if (!elementToFocus) {
69+
// Fall back to the first keyboard-navigable item in the section picker popup
70+
let sectionPickerPopup = document.getElementById("sectionPickerContainer");
71+
if (sectionPickerPopup) {
72+
elementToFocus = sectionPickerPopup.querySelector("[tabindex]:not([tabindex=\"-1\"])") as HTMLElement;
73+
}
74+
}
75+
if (elementToFocus) {
76+
elementToFocus.focus();
77+
}
78+
}, 0);
79+
}
5980
}
6081

6182
// Returns true if successful; false otherwise

src/tests/clipperUI/components/sectionPicker_tests.tsx

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,104 @@ export class SectionPickerTests extends TestModule {
272272
let actual = SectionPickerClass.formatSectionInfoForStorage([]);
273273
strictEqual(actual, undefined, "The section info should be formatted correctly");
274274
});
275+
276+
test("onPopupToggle should focus the currently selected section element when the popup opens and a curSection is set", (assert: QUnitAssert) => {
277+
let done = assert.async();
278+
let clock = sinon.useFakeTimers();
279+
280+
let clipperState = MockProps.getMockClipperState();
281+
let mockNotebooks = MockProps.getMockNotebooks();
282+
let mockSection = {
283+
section: mockNotebooks[0].sections[0],
284+
path: "Clipper Test > Full Page",
285+
parentId: mockNotebooks[0].id
286+
};
287+
initializeClipperStorage(JSON.stringify(mockNotebooks), JSON.stringify(mockSection));
288+
289+
let component = <SectionPicker onPopupToggle={() => {}} clipperState={clipperState} />;
290+
let controllerInstance = MithrilUtils.mountToFixture(component);
291+
292+
// Create a fake section element in the DOM that matches the selected section id
293+
let sectionElement = document.createElement("li");
294+
sectionElement.id = mockSection.section.id;
295+
sectionElement.tabIndex = 70;
296+
let focusCalled = false;
297+
sectionElement.focus = () => { focusCalled = true; };
298+
document.body.appendChild(sectionElement);
299+
300+
controllerInstance.onPopupToggle(true);
301+
clock.tick(0);
302+
303+
ok(focusCalled, "The selected section element should have been focused when the popup opens");
304+
305+
document.body.removeChild(sectionElement);
306+
clock.restore();
307+
done();
308+
});
309+
310+
test("onPopupToggle should focus the first focusable item in the picker popup when the popup opens and no curSection is set", (assert: QUnitAssert) => {
311+
let done = assert.async();
312+
let clock = sinon.useFakeTimers();
313+
314+
let clipperState = MockProps.getMockClipperState();
315+
initializeClipperStorage(undefined, undefined);
316+
317+
let component = <SectionPicker onPopupToggle={() => {}} clipperState={clipperState} />;
318+
let controllerInstance = MithrilUtils.mountToFixture(component);
319+
320+
// Create a fake popup container and a focusable item inside it
321+
let sectionPickerPopup = document.createElement("div");
322+
sectionPickerPopup.id = "sectionPickerContainer";
323+
let firstItem = document.createElement("li");
324+
firstItem.tabIndex = 70;
325+
let focusCalled = false;
326+
firstItem.focus = () => { focusCalled = true; };
327+
sectionPickerPopup.appendChild(firstItem);
328+
document.body.appendChild(sectionPickerPopup);
329+
330+
controllerInstance.onPopupToggle(true);
331+
clock.tick(0);
332+
333+
ok(focusCalled, "The first focusable item in the picker popup should have been focused when no section is selected");
334+
335+
document.body.removeChild(sectionPickerPopup);
336+
clock.restore();
337+
done();
338+
});
339+
340+
test("onPopupToggle should not change focus when the popup closes", (assert: QUnitAssert) => {
341+
let done = assert.async();
342+
let clock = sinon.useFakeTimers();
343+
344+
let clipperState = MockProps.getMockClipperState();
345+
let mockNotebooks = MockProps.getMockNotebooks();
346+
let mockSection = {
347+
section: mockNotebooks[0].sections[0],
348+
path: "Clipper Test > Full Page",
349+
parentId: mockNotebooks[0].id
350+
};
351+
initializeClipperStorage(JSON.stringify(mockNotebooks), JSON.stringify(mockSection));
352+
353+
let component = <SectionPicker onPopupToggle={() => {}} clipperState={clipperState} />;
354+
let controllerInstance = MithrilUtils.mountToFixture(component);
355+
356+
// Create a fake section element to catch any unexpected focus calls
357+
let sectionElement = document.createElement("li");
358+
sectionElement.id = mockSection.section.id;
359+
sectionElement.tabIndex = 70;
360+
let focusCalled = false;
361+
sectionElement.focus = () => { focusCalled = true; };
362+
document.body.appendChild(sectionElement);
363+
364+
controllerInstance.onPopupToggle(false);
365+
clock.tick(0);
366+
367+
ok(!focusCalled, "No focus change should occur when the popup closes");
368+
369+
document.body.removeChild(sectionElement);
370+
clock.restore();
371+
done();
372+
});
275373
}
276374
}
277375

0 commit comments

Comments
 (0)