From 03bd4c5812e9290c85d9bddb955c2fbcbb4a1fa7 Mon Sep 17 00:00:00 2001 From: Ned Redmond Date: Thu, 30 Jan 2025 11:09:14 -0500 Subject: [PATCH] Revert popover bug fix (#2447) * Revert "Fix bug in popover child tab indices (#2443)" This reverts commit c9b03476d3a0aa212a65c8f1427ad5b3649ff28a. * docs(changeset): Reverts changes to focus manager. Doesn't do what we need it to do, and would require further adjustments or changes to Perseus widgets. Better to just wait for an updated implementation. --- .changeset/slow-kings-march.md | 5 + .../__tests__/focus-manager.test.tsx | 104 ------------------ .../src/components/focus-manager.tsx | 7 +- 3 files changed, 7 insertions(+), 109 deletions(-) create mode 100644 .changeset/slow-kings-march.md diff --git a/.changeset/slow-kings-march.md b/.changeset/slow-kings-march.md new file mode 100644 index 0000000000..02af30e4eb --- /dev/null +++ b/.changeset/slow-kings-march.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/wonder-blocks-popover": patch +--- + +Reverts changes to focus manager. Doesn't do what we need it to do, and would require further adjustments or changes to Perseus widgets. Better to just wait for an updated implementation. diff --git a/packages/wonder-blocks-popover/src/components/__tests__/focus-manager.test.tsx b/packages/wonder-blocks-popover/src/components/__tests__/focus-manager.test.tsx index 8c45b653dc..6c0e2505fe 100644 --- a/packages/wonder-blocks-popover/src/components/__tests__/focus-manager.test.tsx +++ b/packages/wonder-blocks-popover/src/components/__tests__/focus-manager.test.tsx @@ -177,108 +177,4 @@ describe("FocusManager", () => { // Assert expect(focusableElementInside).toHaveAttribute("tabIndex", "-1"); }); - - it("changeFocusabilityInsidePopover(true) should keep the original tabindex when explicitly set to 0", async () => { - // Arrange - const externalNodes = ( -
- -
- ); - render(externalNodes); - - // get the anchor reference to be able pass it to the FocusManager - const anchorElementNode = await screen.findByRole("button", { - name: "Open popover", - }); - - render( - -
- -
-
, - ); - - // Act - // focus on the previous element before the popover (anchor element) - anchorElementNode.focus(); - - const firstFocusableElementInside = await screen.findByText( - "first focusable element inside", - ); - - // Assert - expect(firstFocusableElementInside).toHaveAttribute("tabIndex", "0"); - }); - - it("changeFocusabilityInsidePopover(true) should keep the original tabindex when explicitly set to -1", async () => { - // Arrange - const externalNodes = ( -
- -
- ); - render(externalNodes); - - // get the anchor reference to be able pass it to the FocusManager - const anchorElementNode = await screen.findByRole("button", { - name: "Open popover", - }); - - render( - -
- -
-
, - ); - - // Act - // focus on the previous element before the popover (anchor element) - anchorElementNode.focus(); - - const firstFocusableElementInside = await screen.findByText( - "first focusable(...?) element inside", - ); - - // Assert - expect(firstFocusableElementInside).toHaveAttribute("tabIndex", "-1"); - }); - - it("changeFocusabilityInsidePopover(true) should add tabindex of 0 when not set", async () => { - // Arrange - const externalNodes = ( -
- -
- ); - render(externalNodes); - - // get the anchor reference to be able pass it to the FocusManager - const anchorElementNode = await screen.findByRole("button", { - name: "Open popover", - }); - - render( - -
- -
-
, - ); - - // Act - // focus on the previous element before the popover (anchor element) - anchorElementNode.focus(); - - const firstFocusableElementInside = await screen.findByText( - "first focusable element inside", - ); - - // Assert - expect(firstFocusableElementInside).toHaveAttribute("tabIndex", "0"); - }); }); diff --git a/packages/wonder-blocks-popover/src/components/focus-manager.tsx b/packages/wonder-blocks-popover/src/components/focus-manager.tsx index dddf580056..edccdc7dfd 100644 --- a/packages/wonder-blocks-popover/src/components/focus-manager.tsx +++ b/packages/wonder-blocks-popover/src/components/focus-manager.tsx @@ -275,14 +275,11 @@ export default class FocusManager extends React.Component { * reaches to the last focusable element within the document. */ changeFocusabilityInsidePopover = (enabled = true) => { + const tabIndex = enabled ? "0" : "-1"; + // Enable/disable focusability for all the focusable elements inside the // popover. this.elementsThatCanBeFocusableInsidePopover.forEach((element) => { - // If enabled, use the original tabIndex value; otherwise, set it to - // -1 to prevent the element from being focused. - const tabIndex = enabled - ? element.getAttribute("tabIndex") ?? "0" - : "-1"; element.setAttribute("tabIndex", tabIndex); }); };