From 2458673576e7fa791ede27bd93ae5e43cac2dc53 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Thu, 12 Mar 2026 10:34:51 +0000 Subject: [PATCH 1/4] fix(a11y): Focus on selected item when location dropdown opens via keyboard When the location dropdown is expanded using keyboard (Tab + Enter), focus should automatically move to the currently selected section. This improves keyboard accessibility by ensuring users can immediately identify which item is selected. The fix adds focus management to the onPopupToggle handler that: - Focuses on the selected section using aria-selected, data-id, or id - Falls back to focusing the first focusable item if no selection exists Fixes #640 Co-authored-by: KethanaReddy7 Co-Authored-By: Claude Opus 4.6 --- .../clipperUI/components/sectionPicker.tsx | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/src/scripts/clipperUI/components/sectionPicker.tsx b/src/scripts/clipperUI/components/sectionPicker.tsx index 43339d23..5d12a5e9 100644 --- a/src/scripts/clipperUI/components/sectionPicker.tsx +++ b/src/scripts/clipperUI/components/sectionPicker.tsx @@ -54,10 +54,62 @@ export class SectionPickerClass extends ComponentBase { + this.focusOnSelectedSectionInPopup(); + }); } this.props.onPopupToggle(shouldNowBeOpen); } + // Focuses on the currently selected section in the popup for keyboard accessibility + private focusOnSelectedSectionInPopup(): void { + const curSectionId = this.state.curSection && this.state.curSection.section ? this.state.curSection.section.id : undefined; + if (!curSectionId) { + // No section selected, try to focus on the first focusable item in the popup + this.focusOnFirstItemInPopup(); + return; + } + + // Try to find the selected section by aria-selected attribute + let selectedElement = document.querySelector('[aria-selected="true"]') as HTMLElement; + + // If not found, try to find by data-id attribute matching the current section ID + if (!selectedElement) { + selectedElement = document.querySelector(`[data-id="${curSectionId}"]`) as HTMLElement; + } + + // If not found, try to find by id attribute matching the current section ID + if (!selectedElement) { + selectedElement = document.getElementById(curSectionId); + } + + if (selectedElement) { + // Ensure the element is focusable + if (!selectedElement.hasAttribute("tabindex")) { + selectedElement.setAttribute("tabindex", "-1"); + } + selectedElement.focus(); + } else { + // Fallback: focus on the first focusable item in the popup + this.focusOnFirstItemInPopup(); + } + } + + // Fallback method to focus on the first focusable item in the popup + private focusOnFirstItemInPopup(): void { + const popup = document.querySelector(".SectionPickerPopup") as HTMLElement; + if (popup) { + // Find the first focusable element within the popup + const focusableElement = popup.querySelector('a, button, [tabindex]:not([tabindex="-1"]), [role="option"], [role="treeitem"]') as HTMLElement; + if (focusableElement) { + focusableElement.focus(); + } + } + } + // Returns true if successful; false otherwise setDataSource(): boolean { if (!ClipperStateUtilities.isUserLoggedIn(this.props.clipperState)) { From de6419024fb3b72eae8723ac8dbd3b15d9e047d0 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Wed, 18 Mar 2026 04:21:29 +0000 Subject: [PATCH 2/4] fix(a11y): Use setTimeout instead of requestAnimationFrame for popup focus Replace requestAnimationFrame with setTimeout(100ms) to ensure the OneNotePickerComponent popup is fully rendered before attempting to focus on the selected item. requestAnimationFrame alone may fire before the external component has finished rendering its interactive elements. Co-authored-by: KethanaReddy7 --- src/scripts/clipperUI/components/sectionPicker.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/scripts/clipperUI/components/sectionPicker.tsx b/src/scripts/clipperUI/components/sectionPicker.tsx index 5d12a5e9..26c5b1da 100644 --- a/src/scripts/clipperUI/components/sectionPicker.tsx +++ b/src/scripts/clipperUI/components/sectionPicker.tsx @@ -56,10 +56,12 @@ export class SectionPickerClass extends ComponentBase { + // Use setTimeout to ensure the popup has fully rendered before trying to focus. + // requestAnimationFrame alone may fire before the external OneNotePickerComponent + // has finished rendering the popup and its interactive elements. + setTimeout(() => { this.focusOnSelectedSectionInPopup(); - }); + }, 100); } this.props.onPopupToggle(shouldNowBeOpen); } From 6918ceb90c690da5a470cf11748c94a9b89e1e37 Mon Sep 17 00:00:00 2001 From: KethanaReddy7 Date: Wed, 18 Mar 2026 12:21:21 +0530 Subject: [PATCH 3/4] Add focus to location dropdown --- .../clipperUI/components/sectionPicker.tsx | 64 +++--------------- .../components/sectionPicker_tests.tsx | 66 +++++++++++++++++++ 2 files changed, 76 insertions(+), 54 deletions(-) diff --git a/src/scripts/clipperUI/components/sectionPicker.tsx b/src/scripts/clipperUI/components/sectionPicker.tsx index 26c5b1da..e3ac26cd 100644 --- a/src/scripts/clipperUI/components/sectionPicker.tsx +++ b/src/scripts/clipperUI/components/sectionPicker.tsx @@ -54,64 +54,20 @@ export class SectionPickerClass extends ComponentBase { - this.focusOnSelectedSectionInPopup(); - }, 100); + // Move focus to the first item in the dropdown when it opens + requestAnimationFrame(() => { + let notebookList = document.getElementById("notebookList"); + if (notebookList) { + let firstTreeItem = notebookList.querySelector("li[role='treeitem']") as HTMLElement; + if (firstTreeItem) { + firstTreeItem.focus(); + } + } + }); } this.props.onPopupToggle(shouldNowBeOpen); } - // Focuses on the currently selected section in the popup for keyboard accessibility - private focusOnSelectedSectionInPopup(): void { - const curSectionId = this.state.curSection && this.state.curSection.section ? this.state.curSection.section.id : undefined; - if (!curSectionId) { - // No section selected, try to focus on the first focusable item in the popup - this.focusOnFirstItemInPopup(); - return; - } - - // Try to find the selected section by aria-selected attribute - let selectedElement = document.querySelector('[aria-selected="true"]') as HTMLElement; - - // If not found, try to find by data-id attribute matching the current section ID - if (!selectedElement) { - selectedElement = document.querySelector(`[data-id="${curSectionId}"]`) as HTMLElement; - } - - // If not found, try to find by id attribute matching the current section ID - if (!selectedElement) { - selectedElement = document.getElementById(curSectionId); - } - - if (selectedElement) { - // Ensure the element is focusable - if (!selectedElement.hasAttribute("tabindex")) { - selectedElement.setAttribute("tabindex", "-1"); - } - selectedElement.focus(); - } else { - // Fallback: focus on the first focusable item in the popup - this.focusOnFirstItemInPopup(); - } - } - - // Fallback method to focus on the first focusable item in the popup - private focusOnFirstItemInPopup(): void { - const popup = document.querySelector(".SectionPickerPopup") as HTMLElement; - if (popup) { - // Find the first focusable element within the popup - const focusableElement = popup.querySelector('a, button, [tabindex]:not([tabindex="-1"]), [role="option"], [role="treeitem"]') as HTMLElement; - if (focusableElement) { - focusableElement.focus(); - } - } - } - // Returns true if successful; false otherwise setDataSource(): boolean { if (!ClipperStateUtilities.isUserLoggedIn(this.props.clipperState)) { diff --git a/src/tests/clipperUI/components/sectionPicker_tests.tsx b/src/tests/clipperUI/components/sectionPicker_tests.tsx index 00a67e55..45d0aa25 100644 --- a/src/tests/clipperUI/components/sectionPicker_tests.tsx +++ b/src/tests/clipperUI/components/sectionPicker_tests.tsx @@ -882,6 +882,72 @@ export class SectionPickerSinonTests extends TestModule { done(); }); }); + + test("onPopupToggle should move focus to first tree item when dropdown opens", (assert: QUnitAssert) => { + let done = assert.async(); + + let clipperState = MockProps.getMockClipperState(); + let mockNotebooks = MockProps.getMockNotebooks(); + initializeClipperStorage(JSON.stringify(mockNotebooks), undefined, TestConstants.defaultUserInfoAsJsonString); + + let popupToggled = false; + let component = { popupToggled = shouldNowBeOpen; }} + clipperState={clipperState} />; + let controllerInstance = MithrilUtils.mountToFixture(component); + + // Open the dropdown + MithrilUtils.simulateAction(() => { + document.getElementById(TestConstants.Ids.sectionLocationContainer).click(); + }); + + // Wait for requestAnimationFrame to complete + requestAnimationFrame(() => { + let notebookList = document.getElementById("notebookList"); + ok(notebookList, "Notebook list should be present when dropdown is open"); + if (notebookList) { + let firstTreeItem = notebookList.querySelector("li[role='treeitem']") as HTMLElement; + ok(firstTreeItem, "First tree item should exist in the notebook list"); + } + ok(popupToggled, "onPopupToggle should have been called with true"); + done(); + }); + }); + + test("onPopupToggle should call onPopupToggle prop with false when dropdown closes", (assert: QUnitAssert) => { + let done = assert.async(); + + let clipperState = MockProps.getMockClipperState(); + let mockNotebooks = MockProps.getMockNotebooks(); + let mockSection = { + section: mockNotebooks[0].sections[0], + path: "Clipper Test > Full Page", + parentId: mockNotebooks[0].id + }; + initializeClipperStorage(JSON.stringify(mockNotebooks), JSON.stringify(mockSection), TestConstants.defaultUserInfoAsJsonString); + + let lastPopupState: boolean; + let component = { lastPopupState = shouldNowBeOpen; }} + clipperState={clipperState} />; + let controllerInstance = MithrilUtils.mountToFixture(component); + + // Open the dropdown first + MithrilUtils.simulateAction(() => { + document.getElementById(TestConstants.Ids.sectionLocationContainer).click(); + }); + + // Close the dropdown by clicking again + MithrilUtils.simulateAction(() => { + document.getElementById(TestConstants.Ids.sectionLocationContainer).click(); + }); + + // Wait for requestAnimationFrame to complete + requestAnimationFrame(() => { + strictEqual(lastPopupState, false, "onPopupToggle should have been called with false when dropdown closes"); + done(); + }); + }); } } From e0525de7670b9b5c9317eb3930ede2ebbcaf7c3b Mon Sep 17 00:00:00 2001 From: KethanaReddy7 Date: Wed, 18 Mar 2026 12:23:49 +0530 Subject: [PATCH 4/4] update test --- .../components/sectionPicker_tests.tsx | 41 +------------------ 1 file changed, 2 insertions(+), 39 deletions(-) diff --git a/src/tests/clipperUI/components/sectionPicker_tests.tsx b/src/tests/clipperUI/components/sectionPicker_tests.tsx index 45d0aa25..9c0abc85 100644 --- a/src/tests/clipperUI/components/sectionPicker_tests.tsx +++ b/src/tests/clipperUI/components/sectionPicker_tests.tsx @@ -890,11 +890,10 @@ export class SectionPickerSinonTests extends TestModule { let mockNotebooks = MockProps.getMockNotebooks(); initializeClipperStorage(JSON.stringify(mockNotebooks), undefined, TestConstants.defaultUserInfoAsJsonString); - let popupToggled = false; let component = { popupToggled = shouldNowBeOpen; }} + onPopupToggle={() => {}} clipperState={clipperState} />; - let controllerInstance = MithrilUtils.mountToFixture(component); + MithrilUtils.mountToFixture(component); // Open the dropdown MithrilUtils.simulateAction(() => { @@ -909,42 +908,6 @@ export class SectionPickerSinonTests extends TestModule { let firstTreeItem = notebookList.querySelector("li[role='treeitem']") as HTMLElement; ok(firstTreeItem, "First tree item should exist in the notebook list"); } - ok(popupToggled, "onPopupToggle should have been called with true"); - done(); - }); - }); - - test("onPopupToggle should call onPopupToggle prop with false when dropdown closes", (assert: QUnitAssert) => { - let done = assert.async(); - - let clipperState = MockProps.getMockClipperState(); - let mockNotebooks = MockProps.getMockNotebooks(); - let mockSection = { - section: mockNotebooks[0].sections[0], - path: "Clipper Test > Full Page", - parentId: mockNotebooks[0].id - }; - initializeClipperStorage(JSON.stringify(mockNotebooks), JSON.stringify(mockSection), TestConstants.defaultUserInfoAsJsonString); - - let lastPopupState: boolean; - let component = { lastPopupState = shouldNowBeOpen; }} - clipperState={clipperState} />; - let controllerInstance = MithrilUtils.mountToFixture(component); - - // Open the dropdown first - MithrilUtils.simulateAction(() => { - document.getElementById(TestConstants.Ids.sectionLocationContainer).click(); - }); - - // Close the dropdown by clicking again - MithrilUtils.simulateAction(() => { - document.getElementById(TestConstants.Ids.sectionLocationContainer).click(); - }); - - // Wait for requestAnimationFrame to complete - requestAnimationFrame(() => { - strictEqual(lastPopupState, false, "onPopupToggle should have been called with false when dropdown closes"); done(); }); });