A11y Bug 6210624: Keyboard focus and arrow key navigation in location dropdown

src/scripts/clipperUI/components/sectionPicker.tsx

@@ -66,6 +66,62 @@ export class SectionPickerClass extends ComponentBase<SectionPickerState, Sectio
 			});
 		}
 		this.props.onPopupToggle(shouldNowBeOpen);
+		if (shouldNowBeOpen) {
+			// After the popup renders, move keyboard focus to the currently selected section item
+			// so that keyboard users can identify which item is selected and navigate from there.
+			// Also attach an arrow key handler so keyboard users can navigate the list with Up/Down.
+			setTimeout(() => {
+				let sectionPickerPopup = document.getElementById("sectionPickerContainer");
+
+				let curSectionId = this.state.curSection && this.state.curSection.section ? this.state.curSection.section.id : undefined;
+				let elementToFocus: HTMLElement;
+				if (curSectionId) {
+					elementToFocus = document.getElementById(curSectionId) as HTMLElement;
+				}
+				if (!elementToFocus && sectionPickerPopup) {
+					// Fall back to the first keyboard-navigable item in the section picker popup
+					elementToFocus = sectionPickerPopup.querySelector("[tabindex]:not([tabindex=\"-1\"])") as HTMLElement;
+				}
+				if (elementToFocus) {
+					elementToFocus.focus();
+				}
+
+				// Attach Up/Down arrow key navigation for the popup list.
+				// The OneNotePicker library only handles Enter/Tab, so we add arrow key support here.
+				// The listener is attached to the popup element which is removed from the DOM when the
+				// popup closes, so there is no need to explicitly clean it up.
+				// Guard against attaching multiple listeners if onPopupToggle(true) is called more than once.
+				// Use capture so the popup still receives Up/Down events before child controls suppress bubbling.
+				if (sectionPickerPopup && !sectionPickerPopup.getAttribute("data-arrow-key-handler-attached")) {
+					sectionPickerPopup.setAttribute("data-arrow-key-handler-attached", "true");
+					sectionPickerPopup.addEventListener("keydown", (e: KeyboardEvent) => {
+						if (e.which !== Constants.KeyCodes.up && e.which !== Constants.KeyCodes.down) {
+							return;
+						}
+						e.preventDefault();
+						// Only include visible items — exclude elements whose parent is inside a "Closed"
+						// collapsed notebook or sectionGroup (children remain in the DOM but are hidden via CSS).
+						let focusableItems = Array.prototype.slice.call(
+							sectionPickerPopup.querySelectorAll("[tabindex]:not([tabindex=\"-1\"])")
+						).filter((el) => {
+							let parent = (el as HTMLElement).parentElement;
+							return !parent || !parent.closest(".Closed");
+						}) as HTMLElement[];
+						if (focusableItems.length === 0) {
+							return;
+						}
+						let currentIndex = focusableItems.indexOf(document.activeElement as HTMLElement);
+						if (e.which === Constants.KeyCodes.up) {
+							let prevIndex = currentIndex <= 0 ? 0 : currentIndex - 1;
+							focusableItems[prevIndex].focus();
+						} else {
+							let nextIndex = currentIndex >= focusableItems.length - 1 ? focusableItems.length - 1 : currentIndex + 1;
+							focusableItems[nextIndex].focus();
+						}
+					}, true);
+				}
+			}, 0);
+		}
 	}
 
 	// Returns true if successful; false otherwise

src/tests/clipperUI/components/sectionPicker_tests.tsx

@@ -273,42 +273,241 @@ export class SectionPickerTests extends TestModule {
 			strictEqual(actual, undefined, "The section info should be formatted correctly");
 		});
 
-		test("onPopupToggle should restore focus to sectionLocationContainer when popup closes", (assert: QUnitAssert) => {
+		test("onPopupToggle should focus the currently selected section element when the popup opens and a curSection is set", (assert: QUnitAssert) => {
 			let done = assert.async();
+			let clock = sinon.useFakeTimers();
+
 			let clipperState = MockProps.getMockClipperState();
 			let mockNotebooks = MockProps.getMockNotebooks();
-			initializeClipperStorage(JSON.stringify(mockNotebooks), undefined);
+			let mockSection = {
+				section: mockNotebooks[0].sections[0],
+				path: "Clipper Test > Full Page",
+				parentId: mockNotebooks[0].id
+			};
+			initializeClipperStorage(JSON.stringify(mockNotebooks), JSON.stringify(mockSection));
 
-			let popupToggleCalled = false;
-			let component = <SectionPicker
-				onPopupToggle={() => { popupToggleCalled = true; }}
-				clipperState={clipperState} />;
+			let component = <SectionPicker onPopupToggle={() => {}} clipperState={clipperState} />;
 			let controllerInstance = MithrilUtils.mountToFixture(component);
 
-			// Open the popup first
-			MithrilUtils.simulateAction(() => {
-				document.getElementById(TestConstants.Ids.sectionLocationContainer).click();
-			});
+			// Create a fake section element in the DOM that matches the selected section id
+			let sectionElement = document.createElement("li");
+			sectionElement.id = mockSection.section.id;
+			sectionElement.tabIndex = 70;
+			let focusCalled = false;
+			sectionElement.focus = () => { focusCalled = true; };
+			document.body.appendChild(sectionElement);
+
+			controllerInstance.onPopupToggle(true);
+			clock.tick(0);
+
+			ok(focusCalled, "The selected section element should have been focused when the popup opens");
+
+			document.body.removeChild(sectionElement);
+			clock.restore();
+			done();
+		});
+
+		test("onPopupToggle should focus the first focusable item in the picker popup when the popup opens and no curSection is set", (assert: QUnitAssert) => {
+			let done = assert.async();
+			let clock = sinon.useFakeTimers();
+
+			let clipperState = MockProps.getMockClipperState();
+			initializeClipperStorage(undefined, undefined);
+
+			let component = <SectionPicker onPopupToggle={() => {}} clipperState={clipperState} />;
+			let controllerInstance = MithrilUtils.mountToFixture(component);
+
+			// Create a fake popup container and a focusable item inside it
+			let sectionPickerPopup = document.createElement("div");
+			sectionPickerPopup.id = "sectionPickerContainer";
+			let firstItem = document.createElement("li");
+			firstItem.tabIndex = 70;
+			let focusCalled = false;
+			firstItem.focus = () => { focusCalled = true; };
+			sectionPickerPopup.appendChild(firstItem);
+			document.body.appendChild(sectionPickerPopup);
+
+			controllerInstance.onPopupToggle(true);
+			clock.tick(0);
+
+			ok(focusCalled, "The first focusable item in the picker popup should have been focused when no section is selected");
+
+			document.body.removeChild(sectionPickerPopup);
+			clock.restore();
+			done();
+		});
+
+		test("onPopupToggle should not change focus when the popup closes", (assert: QUnitAssert) => {
+			let done = assert.async();
+			let clock = sinon.useFakeTimers();
+
+			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));
+
+			let component = <SectionPicker onPopupToggle={() => {}} clipperState={clipperState} />;
+			let controllerInstance = MithrilUtils.mountToFixture(component);
 
-			// Focus another element to simulate focus being elsewhere
-			let anotherElement = document.createElement("button");
-			document.body.appendChild(anotherElement);
-			anotherElement.focus();
+			// Create a fake section element to catch any unexpected focus calls
+			let sectionElement = document.createElement("li");
+			sectionElement.id = mockSection.section.id;
+			sectionElement.tabIndex = 70;
+			let focusCalled = false;
+			sectionElement.focus = () => { focusCalled = true; };
+			document.body.appendChild(sectionElement);
 
-			// Close the popup by calling onPopupToggle with false
 			controllerInstance.onPopupToggle(false);
+			clock.tick(0);
 
-			// Wait for the deferred focus (setTimeout) to execute
-			setTimeout(() => {
-				// Verify focus was restored to the location container
-				strictEqual(document.activeElement.id, TestConstants.Ids.sectionLocationContainer,
-					"Focus should be restored to sectionLocationContainer when popup closes");
-				ok(popupToggleCalled, "The parent onPopupToggle callback should have been called");
+			ok(!focusCalled, "No focus change should occur when the popup closes");
 
-				// Clean up
-				document.body.removeChild(anotherElement);
-				done();
-			}, 10);
+			document.body.removeChild(sectionElement);
+			clock.restore();
+			done();
+		});
+
+		test("onPopupToggle should enable Down arrow key to move focus to the next item in the popup", (assert: QUnitAssert) => {
+			let done = assert.async();
+			let clock = sinon.useFakeTimers();
+
+			let clipperState = MockProps.getMockClipperState();
+			initializeClipperStorage(undefined, undefined);
+
+			let component = <SectionPicker onPopupToggle={() => {}} clipperState={clipperState} />;
+			let controllerInstance = MithrilUtils.mountToFixture(component);
+
+			// Create a fake popup with two items
+			let sectionPickerPopup = document.createElement("div");
+			sectionPickerPopup.id = "sectionPickerContainer";
+			let firstItem = document.createElement("li");
+			firstItem.tabIndex = 70;
+			let secondItemFocusCalled = false;
+			let secondItem = document.createElement("li");
+			secondItem.tabIndex = 70;
+			secondItem.focus = () => { secondItemFocusCalled = true; };
+			sectionPickerPopup.appendChild(firstItem);
+			sectionPickerPopup.appendChild(secondItem);
+			document.body.appendChild(sectionPickerPopup);
+
+			controllerInstance.onPopupToggle(true);
+			clock.tick(0);
+
+			// Simulate focus on first item and press Down arrow
+			firstItem.focus();
+			let downKeyEvent = document.createEvent("KeyboardEvent");
+			downKeyEvent.initEvent("keydown", true, true);
+			Object.defineProperty(downKeyEvent, "which", { value: 40 });
+			sectionPickerPopup.dispatchEvent(downKeyEvent);
+
+			ok(secondItemFocusCalled, "Down arrow key should move focus to the next item in the popup");
+
+			document.body.removeChild(sectionPickerPopup);
+			clock.restore();
+			done();
+		});
+
+		test("onPopupToggle should enable Up arrow key to move focus to the previous item in the popup", (assert: QUnitAssert) => {
+			let done = assert.async();
+			let clock = sinon.useFakeTimers();
+
+			let clipperState = MockProps.getMockClipperState();
+			initializeClipperStorage(undefined, undefined);
+
+			let component = <SectionPicker onPopupToggle={() => {}} clipperState={clipperState} />;
+			let controllerInstance = MithrilUtils.mountToFixture(component);
+
+			// Create a fake popup with two items
+			let sectionPickerPopup = document.createElement("div");
+			sectionPickerPopup.id = "sectionPickerContainer";
+			let firstItemFocusCalled = false;
+			let firstItem = document.createElement("li");
+			firstItem.tabIndex = 70;
+			firstItem.focus = () => { firstItemFocusCalled = true; };
+			let secondItem = document.createElement("li");
+			secondItem.tabIndex = 70;
+			sectionPickerPopup.appendChild(firstItem);
+			sectionPickerPopup.appendChild(secondItem);
+			document.body.appendChild(sectionPickerPopup);
+
+			controllerInstance.onPopupToggle(true);
+			clock.tick(0);
+
+			// Simulate focus on second item and press Up arrow
+			secondItem.focus();
+			let upKeyEvent = document.createEvent("KeyboardEvent");
+			upKeyEvent.initEvent("keydown", true, true);
+			Object.defineProperty(upKeyEvent, "which", { value: 38 });
+			sectionPickerPopup.dispatchEvent(upKeyEvent);
+
+			ok(firstItemFocusCalled, "Up arrow key should move focus to the previous item in the popup");
+
+			document.body.removeChild(sectionPickerPopup);
+			clock.restore();
+			done();
+		});
+
+		test("onPopupToggle should skip hidden items inside closed notebooks when navigating with Down arrow", (assert: QUnitAssert) => {
+			let done = assert.async();
+			let clock = sinon.useFakeTimers();
+
+			let clipperState = MockProps.getMockClipperState();
+			initializeClipperStorage(undefined, undefined);
+
+			let component = <SectionPicker onPopupToggle={() => {}} clipperState={clipperState} />;
+			let controllerInstance = MithrilUtils.mountToFixture(component);
+
+			// Build a structure that mirrors the OneNotePicker DOM:
+			//   sectionPickerContainer
+			//     li.Notebook.Closed (notebook1, tabindex)
+			//       ul
+			//         li.Section (hiddenSection, tabindex) -- inside closed notebook
+			//     li.Notebook.Opened (notebook2, tabindex)
+			let sectionPickerPopup = document.createElement("div");
+			sectionPickerPopup.id = "sectionPickerContainer";
+
+			let notebook1 = document.createElement("li");
+			notebook1.className = "Notebook Closed";
+			notebook1.tabIndex = 70;
+			let closedChildList = document.createElement("ul");
+			let hiddenSection = document.createElement("li");
+			hiddenSection.className = "Section";
+			hiddenSection.tabIndex = 70;
+			let hiddenSectionFocusCalled = false;
+			hiddenSection.focus = () => { hiddenSectionFocusCalled = true; };
+			closedChildList.appendChild(hiddenSection);
+			notebook1.appendChild(closedChildList);
+
+			let notebook2FocusCalled = false;
+			let notebook2 = document.createElement("li");
+			notebook2.className = "Notebook Opened";
+			notebook2.tabIndex = 70;
+			notebook2.focus = () => { notebook2FocusCalled = true; };
+
+			sectionPickerPopup.appendChild(notebook1);
+			sectionPickerPopup.appendChild(notebook2);
+			document.body.appendChild(sectionPickerPopup);
+
+			controllerInstance.onPopupToggle(true);
+			clock.tick(0);
+
+			// Press Down arrow from notebook1 — should jump to notebook2, skipping the hidden section inside notebook1
+			notebook1.focus();
+			let downKeyEvent = document.createEvent("KeyboardEvent");
+			downKeyEvent.initEvent("keydown", true, true);
+			Object.defineProperty(downKeyEvent, "which", { value: 40 });
+			sectionPickerPopup.dispatchEvent(downKeyEvent);
+
+			ok(!hiddenSectionFocusCalled, "Hidden section inside a closed notebook should not receive focus");
+			ok(notebook2FocusCalled, "Down arrow from a closed notebook should move focus to the next visible notebook");
+
+			document.body.removeChild(sectionPickerPopup);
+			clock.restore();
+			done();
 		});
 	}
 }