From f50ccab10251d31498bde3e36d7dabf529705e2f Mon Sep 17 00:00:00 2001 From: Alireza Date: Sun, 8 Dec 2024 00:41:15 +0100 Subject: [PATCH] fix(Tree): fix a regression in keyboard navigation Since @react-aria/selection@3.16.0 getKeyLeftOf and getKeyRightOf is removed if orientation is vertical and layout is "stack". Using the single-argument overload of the base constructor doesn't set a default value for orientation (which seems a little random), which in turn makes the if statement that removes the methods not run. https://github.com/adobe/react-spectrum/blob/8228e4efd9be99973058a1f90fc7f7377e673f78/packages/%40react-aria/selection/src/ListKeyboardDelegate.ts#L67 --- .../SpeedSearchTree/SpeedSearchTree.cy.tsx | 35 +++++++++++++++++++ packages/jui/src/Tree/Tree.cy.tsx | 35 +++++++++++++++++++ .../jui/src/Tree/TreeKeyboardDelegate.tsx | 12 +++++-- 3 files changed, 80 insertions(+), 2 deletions(-) diff --git a/packages/jui/src/Tree/SpeedSearchTree/SpeedSearchTree.cy.tsx b/packages/jui/src/Tree/SpeedSearchTree/SpeedSearchTree.cy.tsx index da1e272a..d911a7e9 100644 --- a/packages/jui/src/Tree/SpeedSearchTree/SpeedSearchTree.cy.tsx +++ b/packages/jui/src/Tree/SpeedSearchTree/SpeedSearchTree.cy.tsx @@ -9,6 +9,41 @@ const { Dynamic, HighlightsWithSpace } = composeStories(stories); const OS_NORMALIZED_META = Cypress.platform === "darwin" ? "Meta" : "Control"; describe("SpeedSearchTree", () => { + it("expands/collapses and navigates nodes by arrow keys", () => { + cy.mount( + + + node 1.1 + node 1.2 + + + ); + + cy.findByRole("treeitem") // only one should be visible initially + .click() // focus + .realPress("ArrowRight"); + // node 1 should be expanded + cy.findByRole("treeitem", { name: "node 1.1" }); + cy.findByRole("treeitem", { name: "node 1.2" }); + // but selection doesn't go to the children right away + cy.findByRole("treeitem", { name: "node 1", selected: true }); + cy.realPress("ArrowDown"); + cy.findByRole("treeitem", { name: "node 1.1", selected: true }); + cy.realPress("ArrowDown"); + cy.findByRole("treeitem", { name: "node 1.2", selected: true }); + cy.realPress("ArrowLeft"); // selection should move to node 1 but it remains expanded + cy.findByRole("treeitem", { name: "node 1", selected: true }); + cy.realPress("ArrowLeft"); // the second ArrowLeft closes the node + cy.findAllByRole("treeitem").should("have.length", 1); + + cy.realPress("ArrowRight"); // expanding the node again and going down with ArrowRight this time + cy.findByRole("treeitem", { name: "node 1", selected: true }); + cy.realPress("ArrowRight"); + cy.findByRole("treeitem", { name: "node 1.1", selected: true }); + cy.realPress("ArrowRight"); + cy.findByRole("treeitem", { name: "node 1.2", selected: true }); + }); + it("supports Speed Search in dynamic items mode", () => { cy.mount(); diff --git a/packages/jui/src/Tree/Tree.cy.tsx b/packages/jui/src/Tree/Tree.cy.tsx index 238f6706..1e414ec8 100644 --- a/packages/jui/src/Tree/Tree.cy.tsx +++ b/packages/jui/src/Tree/Tree.cy.tsx @@ -8,6 +8,41 @@ import { Item } from "../Collections"; const { Static, ScrollAndContainerWidth } = composeStories(stories); describe("Tree", () => { + it("expands/collapses and navigates nodes by arrow keys", () => { + cy.mount( + + + node 1.1 + node 1.2 + + + ); + + cy.findByRole("treeitem") // only one should be visible initially + .click() // focus + .realPress("ArrowRight"); + // node 1 should be expanded + cy.findByRole("treeitem", { name: "node 1.1" }); + cy.findByRole("treeitem", { name: "node 1.2" }); + // but selection doesn't go to the children right away + cy.findByRole("treeitem", { name: "node 1", selected: true }); + cy.realPress("ArrowDown"); + cy.findByRole("treeitem", { name: "node 1.1", selected: true }); + cy.realPress("ArrowDown"); + cy.findByRole("treeitem", { name: "node 1.2", selected: true }); + cy.realPress("ArrowLeft"); // selection should move to node 1 but it remains expanded + cy.findByRole("treeitem", { name: "node 1", selected: true }); + cy.realPress("ArrowLeft"); // the second ArrowLeft closes the node + cy.findAllByRole("treeitem").should("have.length", 1); + + cy.realPress("ArrowRight"); // expanding the node again and going down with ArrowRight this time + cy.findByRole("treeitem", { name: "node 1", selected: true }); + cy.realPress("ArrowRight"); + cy.findByRole("treeitem", { name: "node 1.1", selected: true }); + cy.realPress("ArrowRight"); + cy.findByRole("treeitem", { name: "node 1.2", selected: true }); + }); + it("opens nested expandable single-child items", () => { cy.mount(); diff --git a/packages/jui/src/Tree/TreeKeyboardDelegate.tsx b/packages/jui/src/Tree/TreeKeyboardDelegate.tsx index fed961d9..c6baa8a3 100644 --- a/packages/jui/src/Tree/TreeKeyboardDelegate.tsx +++ b/packages/jui/src/Tree/TreeKeyboardDelegate.tsx @@ -5,11 +5,19 @@ import React, { Key, RefObject } from "react"; export class TreeKeyboardDelegate extends ListKeyboardDelegate { constructor( private collection: Collection>, - private disabledKeys: Set, + disabledKeys: Set, ref: RefObject, collator?: Intl.Collator ) { - super(collection, disabledKeys, ref, collator); + super({ + collection, + disabledKeys, + ref, + collator, + // Since @react-aria/selection@3.16.0 getKeyLeftOf and getKeyRightOf is + // removed if orientation is vertical and layout is "stack". + layout: "grid", + }); } getKeyLeftOf(key: React.Key): React.Key {