From 20994f3fe1fef2e5902d3b0eb8fcf505578e033c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ademir=20Jos=C3=A9=20Ferreira=20J=C3=BAnior?= Date: Sun, 2 Aug 2026 02:36:14 -0300 Subject: [PATCH 1/5] Carry sibling position and parent linkage on navigator rows A tree reports position within its own level, so `aria-setsize` and `aria-posinset` cannot come from the flat row index. Parent linkage is what traversal to an ancestor row needs. --- .../utils/articleNavigatorRows.test.ts | 63 ++++++++++++++ .../utils/articleNavigatorRows.ts | 85 ++++++++++++++----- 2 files changed, 127 insertions(+), 21 deletions(-) diff --git a/src/features/folder-context/utils/articleNavigatorRows.test.ts b/src/features/folder-context/utils/articleNavigatorRows.test.ts index ebcaa67..7b2fefd 100644 --- a/src/features/folder-context/utils/articleNavigatorRows.test.ts +++ b/src/features/folder-context/utils/articleNavigatorRows.test.ts @@ -48,14 +48,20 @@ describe("article navigator rows", () => { hasChildren: true, isExpanded: true, name: "docs", + parentIndex: null, path: "C:/Notes/docs", + posInSet: 1, + setSize: 3, }, { kind: "file", depth: 1, isActive: true, name: "guide.md", + parentIndex: 0, path: "C:/Notes/docs/guide.md", + posInSet: 1, + setSize: 1, }, { kind: "directory", @@ -63,18 +69,75 @@ describe("article navigator rows", () => { hasChildren: true, isExpanded: false, name: "drafts", + parentIndex: null, path: "C:/Notes/drafts", + posInSet: 2, + setSize: 3, }, { kind: "file", depth: 0, isActive: false, name: "readme.md", + parentIndex: null, path: "C:/Notes/readme.md", + posInSet: 3, + setSize: 3, }, ]); }); + it("scopes position and size to siblings under the same parent", () => { + expect( + buildArticleNavigatorRows({ + activeArticlePath: null, + expandedDirectoryPaths: ["C:/Notes/docs", "C:/Notes/drafts", "C:/Notes/drafts/archive"], + tree: articleTree, + }).map(({ depth, name, parentIndex, posInSet, setSize }) => ({ + depth, + name, + parentIndex, + posInSet, + setSize, + })), + ).toEqual([ + { depth: 0, name: "docs", parentIndex: null, posInSet: 1, setSize: 3 }, + { depth: 1, name: "guide.md", parentIndex: 0, posInSet: 1, setSize: 1 }, + { depth: 0, name: "drafts", parentIndex: null, posInSet: 2, setSize: 3 }, + { depth: 1, name: "archive", parentIndex: 2, posInSet: 1, setSize: 1 }, + { depth: 2, name: "old.md", parentIndex: 3, posInSet: 1, setSize: 1 }, + { depth: 0, name: "readme.md", parentIndex: null, posInSet: 3, setSize: 3 }, + ]); + }); + + it("keeps sibling size independent of how many descendants are expanded", () => { + const collapsedRows = buildArticleNavigatorRows({ + activeArticlePath: null, + expandedDirectoryPaths: [], + tree: articleTree, + }); + const expandedRows = buildArticleNavigatorRows({ + activeArticlePath: null, + expandedDirectoryPaths: ["C:/Notes/docs"], + tree: articleTree, + }); + + expect(collapsedRows.map(({ posInSet, setSize }) => [posInSet, setSize])).toEqual([ + [1, 3], + [2, 3], + [3, 3], + ]); + expect( + expandedRows + .filter(({ depth }) => depth === 0) + .map(({ posInSet, setSize }) => [posInSet, setSize]), + ).toEqual([ + [1, 3], + [2, 3], + [3, 3], + ]); + }); + it("collects directory paths in tree order", () => { expect(getArticleDirectoryPaths(articleTree)).toEqual([ "C:/Notes/docs", diff --git a/src/features/folder-context/utils/articleNavigatorRows.ts b/src/features/folder-context/utils/articleNavigatorRows.ts index aafaafe..3b6fb28 100644 --- a/src/features/folder-context/utils/articleNavigatorRows.ts +++ b/src/features/folder-context/utils/articleNavigatorRows.ts @@ -6,7 +6,10 @@ import type { ArticleTree, ArticleTreeNode } from "../services/folderContext"; interface ArticleNavigatorRowBase { depth: number; name: string; + parentIndex: number | null; path: string; + posInSet: number; + setSize: number; } export interface ArticleNavigatorDirectoryRow extends ArticleNavigatorRowBase { @@ -34,31 +37,40 @@ export const buildArticleNavigatorRows = ({ tree, }: BuildArticleNavigatorRowsOptions): ArticleNavigatorRow[] => { const expandedDirectoryPathSet = new PathSet(expandedDirectoryPaths); - - return flattenTree({ + const entries = flattenTree({ getChildren: getArticleTreeNodeChildren, roots: tree.children, shouldTraverseChildren: ({ node }) => node.kind === "directory" && expandedDirectoryPathSet.has(node.path), - }).map( - ({ depth, node }): ArticleNavigatorRow => - node.kind === "file" - ? { - kind: "file", - depth, - isActive: activeArticlePath ? isSamePath(node.path, activeArticlePath) : false, - name: node.name, - path: node.path, - } - : { - kind: "directory", - depth, - hasChildren: node.children.length > 0, - isExpanded: expandedDirectoryPathSet.has(node.path), - name: node.name, - path: node.path, - }, - ); + }); + const positions = getTreePositions(entries.map(({ depth }) => depth)); + + return entries.map(({ depth, node }, index): ArticleNavigatorRow => { + const { parentIndex, posInSet, setSize } = positions[index]; + + return node.kind === "file" + ? { + kind: "file", + depth, + isActive: activeArticlePath ? isSamePath(node.path, activeArticlePath) : false, + name: node.name, + parentIndex, + path: node.path, + posInSet, + setSize, + } + : { + kind: "directory", + depth, + hasChildren: node.children.length > 0, + isExpanded: expandedDirectoryPathSet.has(node.path), + name: node.name, + parentIndex, + path: node.path, + posInSet, + setSize, + }; + }); }; export const getArticleDirectoryPaths = (tree: ArticleTree) => @@ -79,3 +91,34 @@ export const getArticleAncestorDirectoryPaths = ( const getArticleTreeNodeChildren = (node: ArticleTreeNode) => node.kind === "directory" ? node.children : []; + +interface ArticleNavigatorRowTreePosition { + parentIndex: number | null; + posInSet: number; + setSize: number; +} + +const getTreePositions = (depths: number[]): ArticleNavigatorRowTreePosition[] => { + const openAncestorIndexes: number[] = []; + const parentIndexes = depths.map((depth, index) => { + // Rows arrive depth first, so anything recorded deeper is a closed subtree. + openAncestorIndexes.length = depth; + openAncestorIndexes[depth] = index; + + return depth === 0 ? null : openAncestorIndexes[depth - 1]; + }); + + const siblingCounts = new Map(); + for (const parentIndex of parentIndexes) { + siblingCounts.set(parentIndex, (siblingCounts.get(parentIndex) ?? 0) + 1); + } + + const takenPositions = new Map(); + + return parentIndexes.map((parentIndex) => { + const posInSet = (takenPositions.get(parentIndex) ?? 0) + 1; + takenPositions.set(parentIndex, posInSet); + + return { parentIndex, posInSet, setSize: siblingCounts.get(parentIndex) ?? 0 }; + }); +}; From 5112aa3b28d3e45f149e654e47b7d61dd2e5afca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ademir=20Jos=C3=A9=20Ferreira=20J=C3=BAnior?= Date: Sun, 2 Aug 2026 02:43:15 -0300 Subject: [PATCH 2/5] Expose the article navigator as a tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `
  • ` is the `treeitem` rather than a wrapper around a button, so nothing sits between the tree and its items and the tree owns row keys outright — a native button would activate on `Enter` and `Space` behind its back. Empty directories stop being disabled buttons. `disabled` took them out of the tab order entirely, which is the opposite of what an unselectable but real folder should do. --- CHANGELOG.md | 2 + docs/decisions.md | 14 +++ src/components/layout/Shell.test.tsx | 30 +++-- src/components/ui/VirtualList.tsx | 9 +- .../components/ArticleNavigator.test.tsx | 110 ++++++++++++++++- .../components/ArticleNavigator.tsx | 111 +++++++++++------- 6 files changed, 218 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 96ee8ff..73b7872 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ Leafdown uses lightweight [Keep a Changelog](https://keepachangelog.com/en/1.1.0 ### Fixed +- Announce the article navigator as a tree, with the nesting depth, sibling position, and expanded state of every row. +- Keep empty folders in the article navigator reachable instead of skipping them. - Open the editor context popup with `Shift+F10` or the `Menu` key and operate every command in it from the keyboard. - Announce the editor context popup as a named toolbar instead of an unnamed dialog. - Announce recent files and recent folders under their own headings in the `Open recent` menu. diff --git a/docs/decisions.md b/docs/decisions.md index 5984682..0b6b061 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -249,6 +249,20 @@ - Extending validation is a local change with no dependency surface, and its type-checking cost stays proportional to the shapes actually declared. - The salvage and repair behavior is Leafdown's to maintain and test. +### Expose the article navigator as a flattened tree + +**Decision:** The article navigator is an ARIA `tree`, flattened rather than nested: the scrolling list carries `role="tree"`, every row is a `treeitem` child of it, and depth travels on `aria-level` with `role="group"` omitted. Selection does not follow focus — `aria-selected` marks the open document, and only `Enter`, `Space`, and click open one. + +**Rationale:** Hierarchy has to be announced, not just indented, and a flat list of buttons has nowhere to put nesting, position, or expanded state. The nested `role="group"` markup the pattern usually shows cannot be produced here, because virtualization keeps only a window of rows in the DOM and a group wrapper would have to enclose children that do not exist; `aria-level` carries the same relationship without the DOM nesting. Selection following focus would open every document arrowed past, thrashing the editor. + +**Consequences:** + +- `aria-setsize` and `aria-posinset` are scoped to siblings under the same parent and computed in the row model, because a flat row index answers a different question and the DOM holds only a window of rows. +- Every `treeitem` carries `aria-selected`, including directory rows that can never be selected. A tree where only some items carry it has the rest announced as "not selected". +- `aria-current` no longer marks the open document. The `data-active` visual treatment is unchanged. +- The focused row and the selected row are routinely different, which is what file-explorer users expect. +- Rows are `treeitem`s rather than buttons, so their keyboard behavior is the tree's to implement rather than something the platform supplies. + ## Platform Decisions ### Windows first, cross-platform aware diff --git a/src/components/layout/Shell.test.tsx b/src/components/layout/Shell.test.tsx index 7f664b1..1c5b652 100644 --- a/src/components/layout/Shell.test.tsx +++ b/src/components/layout/Shell.test.tsx @@ -77,10 +77,16 @@ describe("Shell", () => { render(); - expect(screen.getByRole("button", { name: "readme.md" })).toBeInTheDocument(); - expect(screen.getByRole("button", { name: "draft.markdown" })).toBeInTheDocument(); - expect(screen.getByRole("button", { name: "docs" })).toHaveAttribute("aria-expanded", "false"); - expect(screen.queryByRole("button", { name: "spec.md" })).not.toBeInTheDocument(); + expect(screen.getByRole("treeitem", { name: "readme.md" })).toBeInTheDocument(); + expect(screen.getByRole("treeitem", { name: "draft.markdown" })).toBeInTheDocument(); + expect(screen.getByRole("treeitem", { name: "docs" })).toHaveAttribute( + "aria-expanded", + "false", + ); + expect(screen.queryByRole("treeitem", { name: "spec.md" })).not.toBeInTheDocument(); + expect(screen.getByRole("complementary", { name: "Article navigator" })).toContainElement( + screen.getByRole("tree", { name: "Articles" }), + ); expect(screen.getByText("No document open")).toBeInTheDocument(); expect( screen.getByText("Select a Markdown file from the sidebar or create a new document."), @@ -100,14 +106,14 @@ describe("Shell", () => { render(); await waitFor(() => { - expect(screen.getByRole("button", { name: "spec.md" })).toHaveAttribute( - "aria-current", - "page", + expect(screen.getByRole("treeitem", { name: "spec.md" })).toHaveAttribute( + "aria-selected", + "true", ); }); - expect(screen.getByRole("button", { name: "docs" })).toHaveAttribute("aria-expanded", "true"); - expect(screen.getByRole("button", { name: "empty" })).toBeDisabled(); + expect(screen.getByRole("treeitem", { name: "docs" })).toHaveAttribute("aria-expanded", "true"); + expect(screen.getByRole("treeitem", { name: "empty" })).not.toHaveAttribute("aria-expanded"); expect(screen.getByTestId("active-document-host")).toHaveTextContent("# Spec"); }); @@ -120,7 +126,7 @@ describe("Shell", () => { expect(screen.getByText("No Markdown files found")).toBeInTheDocument(); expect(screen.getByText("No supported Markdown files found.")).toBeInTheDocument(); - expect(screen.getByRole("button", { name: "nested" })).toBeDisabled(); + expect(screen.getByRole("treeitem", { name: "nested" })).not.toHaveAttribute("aria-expanded"); }); it("hides the sidebar when the persisted sidebar setting is off", () => { @@ -144,7 +150,7 @@ describe("Shell", () => { ); const { user } = renderWithUser(); - await user.click(screen.getByRole("button", { name: "readme.md" })); + await user.click(screen.getByRole("treeitem", { name: "readme.md" })); await waitFor(() => { expect(toast.error).toHaveBeenCalledWith("Could not read Markdown file.", { @@ -162,7 +168,7 @@ describe("Shell", () => { mockTauriApiCommand("openMarkdownFile", () => Promise.reject(OVERSIZED_MARKDOWN_FILE_ERROR)); const { user } = renderWithUser(); - await user.click(screen.getByRole("button", { name: "draft.markdown" })); + await user.click(screen.getByRole("treeitem", { name: "draft.markdown" })); await waitFor(() => { expect(toast.error).toHaveBeenCalledWith("Markdown file is too large.", { diff --git a/src/components/ui/VirtualList.tsx b/src/components/ui/VirtualList.tsx index e8b9479..190ea8b 100644 --- a/src/components/ui/VirtualList.tsx +++ b/src/components/ui/VirtualList.tsx @@ -186,4 +186,11 @@ function VirtualListEmpty({ children }: { children: ReactNode }) { return children; } -export { VirtualList, VirtualListContent, VirtualListEmpty, VirtualListItem, VirtualListItems }; +export { + VirtualList, + VirtualListContent, + VirtualListEmpty, + VirtualListItem, + VirtualListItems, + type VirtualItem, +}; diff --git a/src/features/folder-context/components/ArticleNavigator.test.tsx b/src/features/folder-context/components/ArticleNavigator.test.tsx index cf3747b..a82d94b 100644 --- a/src/features/folder-context/components/ArticleNavigator.test.tsx +++ b/src/features/folder-context/components/ArticleNavigator.test.tsx @@ -1,6 +1,11 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; -import { createEmptyFolderContext, createFolderContext } from "@/test/factories/folderContext"; +import { + createEmptyFolderContext, + createFolderContext, + createNestedArticleTree, +} from "@/test/factories/folderContext"; +import { TEST_NESTED_DIRECTORY_PATH } from "@/test/fixtures/paths"; import { render, renderWithUser, screen } from "@/test/utils/react"; import { useArticleNavigatorStore } from "../stores/articleNavigator"; @@ -8,6 +13,8 @@ import { ArticleNavigator } from "./ArticleNavigator"; const folderContext = createFolderContext(); +const nestedFolderContext = createFolderContext({ tree: createNestedArticleTree() }); + const emptyFolderContext = createEmptyFolderContext(); const folderContextWithScanWarning = createFolderContext({ @@ -33,7 +40,23 @@ describe("ArticleNavigator", () => { />, ); - await user.click(screen.getByRole("button", { name: "readme.md" })); + await user.click(screen.getByRole("treeitem", { name: "readme.md" })); + + expect(onOpenArticle).toHaveBeenCalledWith("C:/Notes/readme.md"); + }); + + it("opens an article from the keyboard", async () => { + const onOpenArticle = vi.fn(); + const { user } = renderWithUser( + , + ); + + screen.getByRole("treeitem", { name: "readme.md" }).focus(); + await user.keyboard("{Enter}"); expect(onOpenArticle).toHaveBeenCalledWith("C:/Notes/readme.md"); }); @@ -48,11 +71,92 @@ describe("ArticleNavigator", () => { />, ); - await user.click(screen.getByRole("button", { name: "readme.md" })); + await user.click(screen.getByRole("treeitem", { name: "readme.md" })); expect(onOpenArticle).not.toHaveBeenCalled(); }); + it("exposes the articles as a tree named apart from the surrounding landmark", () => { + render( + , + ); + + expect(screen.getByRole("tree", { name: "Articles" })).toBeInTheDocument(); + expect(screen.queryByRole("list")).not.toBeInTheDocument(); + }); + + it("reports nesting depth, sibling position, and expanded state on every row", () => { + useArticleNavigatorStore.getState().expandDirectories([TEST_NESTED_DIRECTORY_PATH]); + + render( + , + ); + + expect( + screen + .getAllByRole("treeitem") + .map((row) => [ + row.textContent, + row.getAttribute("aria-level"), + row.getAttribute("aria-posinset"), + row.getAttribute("aria-setsize"), + row.getAttribute("aria-expanded"), + ]), + ).toEqual([ + ["readme.md", "1", "1", "4", null], + ["draft.markdown", "1", "2", "4", null], + ["docs", "1", "3", "4", "true"], + ["spec.md", "2", "1", "1", null], + ["empty", "1", "4", "4", null], + ]); + }); + + it("keeps an empty directory reachable instead of disabling it", () => { + render( + , + ); + + const emptyDirectory = screen.getByRole("treeitem", { name: "empty" }); + emptyDirectory.focus(); + + expect(emptyDirectory).toHaveFocus(); + expect(emptyDirectory).not.toHaveAttribute("aria-disabled"); + }); + + it("marks the open document as the selected row", () => { + render( + , + ); + + expect( + screen + .getAllByRole("treeitem") + .map((row) => [row.textContent, row.getAttribute("aria-selected")]), + ).toEqual([ + ["readme.md", "true"], + ["draft.markdown", "false"], + ["docs", "false"], + ["empty", "false"], + ]); + expect(screen.queryByRole("treeitem", { current: "page" })).not.toBeInTheDocument(); + }); + it("shows when the active document is outside the current folder context", () => { render( - + > {(row, virtualRow) => ( - - {row.kind === "directory" && ( - - )} - {row.kind === "file" && } - + )} @@ -226,25 +229,66 @@ function ArticleNavigatorRows({ ); } -interface DirectoryRowProps { - row: ArticleNavigatorDirectoryRow; +interface ArticleNavigatorTreeItemProps { + onOpenArticle: (path: string) => void; onToggleDirectory: (path: string) => void; + row: ArticleNavigatorRow; + virtualRow: VirtualItem; } -function DirectoryRow({ row, onToggleDirectory }: DirectoryRowProps) { - const Icon = row.isExpanded ? ChevronDownIcon : ChevronRightIcon; +function ArticleNavigatorTreeItem({ + onOpenArticle, + onToggleDirectory, + row, + virtualRow, +}: ArticleNavigatorTreeItemProps) { + const activate = () => + row.kind === "directory" ? onToggleDirectory(row.path) : onOpenArticle(row.path); + + const handleKeyDown = (event: KeyboardEvent) => { + if ((event.key !== "Enter" && event.key !== " ") || !hasNoShortcutModifier(event.nativeEvent)) { + return; + } + + event.preventDefault(); + activate(); + }; return ( - + ); } -interface ArticleRowProps { - row: ArticleNavigatorArticleRow; - onOpenArticle: (path: string) => void; -} - -function ArticleRow({ row, onOpenArticle }: ArticleRowProps) { +function ArticleRowContent({ row }: { row: ArticleNavigatorArticleRow }) { return ( - + ); } From 9d6a483767abd456580f218df2ded57a03604894 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ademir=20Jos=C3=A9=20Ferreira=20J=C3=BAnior?= Date: Sun, 2 Aug 2026 02:52:48 -0300 Subject: [PATCH 3/5] Traverse the article navigator by keyboard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Focus is tracked by path rather than index, because expanding a directory renumbers every row below it. A path that is gone resolves to the deepest surviving ancestor, which is where focus lands when a directory collapses over it. `VirtualList` gains a pinned index so the row holding the tab stop stays rendered when it scrolls out of the window. Unmounting it drops focus to the body and leaves the navigator with no tab stop at all — the defect the tree is meant to fix. --- CHANGELOG.md | 1 + docs/decisions.md | 5 +- docs/specification.md | 12 ++ src/components/ui/VirtualList.test.tsx | 23 +++ src/components/ui/VirtualList.tsx | 22 ++- .../components/ArticleNavigator.test.tsx | 94 ++++++++++++- .../components/ArticleNavigator.tsx | 133 ++++++++++++++---- .../utils/articleNavigatorTraversal.test.ts | 112 +++++++++++++++ .../utils/articleNavigatorTraversal.ts | 102 ++++++++++++++ 9 files changed, 476 insertions(+), 28 deletions(-) create mode 100644 src/components/ui/VirtualList.test.tsx create mode 100644 src/features/folder-context/utils/articleNavigatorTraversal.test.ts create mode 100644 src/features/folder-context/utils/articleNavigatorTraversal.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 73b7872..a08513f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ Leafdown uses lightweight [Keep a Changelog](https://keepachangelog.com/en/1.1.0 ### Fixed +- Traverse the article navigator with the arrow keys, `Home`, and `End`, and pass it with a single `Tab` instead of one per article. - Announce the article navigator as a tree, with the nesting depth, sibling position, and expanded state of every row. - Keep empty folders in the article navigator reachable instead of skipping them. - Open the editor context popup with `Shift+F10` or the `Menu` key and operate every command in it from the keyboard. diff --git a/docs/decisions.md b/docs/decisions.md index 0b6b061..59ded58 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -251,9 +251,9 @@ ### Expose the article navigator as a flattened tree -**Decision:** The article navigator is an ARIA `tree`, flattened rather than nested: the scrolling list carries `role="tree"`, every row is a `treeitem` child of it, and depth travels on `aria-level` with `role="group"` omitted. Selection does not follow focus — `aria-selected` marks the open document, and only `Enter`, `Space`, and click open one. +**Decision:** The article navigator is an ARIA `tree`, flattened rather than nested: the scrolling list carries `role="tree"`, every row is a `treeitem` child of it, one row at a time holds the tab stop, and depth travels on `aria-level` with `role="group"` omitted. Selection does not follow focus — `aria-selected` marks the open document, and only `Enter`, `Space`, and click open one. -**Rationale:** Hierarchy has to be announced, not just indented, and a flat list of buttons has nowhere to put nesting, position, or expanded state. The nested `role="group"` markup the pattern usually shows cannot be produced here, because virtualization keeps only a window of rows in the DOM and a group wrapper would have to enclose children that do not exist; `aria-level` carries the same relationship without the DOM nesting. Selection following focus would open every document arrowed past, thrashing the editor. +**Rationale:** Hierarchy has to be announced, not just indented, and a flat list of buttons has nowhere to put nesting, position, or expanded state. The nested `role="group"` markup the pattern usually shows cannot be produced here, because virtualization keeps only a window of rows in the DOM and a group wrapper would have to enclose children that do not exist; `aria-level` carries the same relationship without the DOM nesting. Selection following focus would open every document arrowed past, thrashing the editor. The tab stop roves across rows rather than resting on the container with `aria-activedescendant`: the active descendant still has to be a rendered row, so that model does not avoid keeping the focused row alive, and it gives up the native focus ring the rows already carry. **Consequences:** @@ -262,6 +262,7 @@ - `aria-current` no longer marks the open document. The `data-active` visual treatment is unchanged. - The focused row and the selected row are routinely different, which is what file-explorer users expect. - Rows are `treeitem`s rather than buttons, so their keyboard behavior is the tree's to implement rather than something the platform supplies. +- The row holding the tab stop has to stay rendered even when it scrolls out of the virtualized window. Unmounting it drops focus to the document body and leaves the navigator with no tab stop at all, which would take the scroll region out of the tab sequence. ## Platform Decisions diff --git a/docs/specification.md b/docs/specification.md index 1ec49b3..61aedaa 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -50,6 +50,18 @@ Primary user interface surfaces: - **Context popup:** provides quick document actions from selection or right-click. - **Modal layer:** presents secondary screens and blocking dialogs outside the main editor surface. +### Article Navigator Traversal + +The article navigator is a tree and takes a single tab stop. Focus enters on the open document, or on the first row when no document is open. + +- `ArrowDown` and `ArrowUp`: Move to the next or previous visible row, stopping at either end. +- `ArrowRight`: Expand the focused directory, or move into it when it is already expanded. +- `ArrowLeft`: Collapse the focused directory, or move to the parent directory when it is already collapsed. +- `Home` and `End`: Move to the first or last visible row. +- `Enter` and `Space`: Open the focused article, or expand and collapse the focused directory. Clicking a row does the same. + +Moving focus never opens a document, so the focused row and the open document are routinely different rows. An empty directory is an ordinary row that can be focused and read, with nothing to expand. Collapsing a directory that contains the focused row moves focus to the nearest row that survives. + ## State Model These state axes compose. A document session, for example, can have a folder context and be saved and clean or dirty. diff --git a/src/components/ui/VirtualList.test.tsx b/src/components/ui/VirtualList.test.tsx new file mode 100644 index 0000000..f69f320 --- /dev/null +++ b/src/components/ui/VirtualList.test.tsx @@ -0,0 +1,23 @@ +import { describe, expect, it } from "vitest"; + +import { withPinnedIndex } from "./VirtualList"; + +describe("withPinnedIndex", () => { + it("leaves the rendered range alone when nothing is pinned", () => { + const indexes = [4, 5, 6]; + + expect(withPinnedIndex(indexes, undefined)).toBe(indexes); + }); + + it("leaves the rendered range alone when the pinned row is already in it", () => { + const indexes = [4, 5, 6]; + + expect(withPinnedIndex(indexes, 5)).toBe(indexes); + }); + + it("adds a pinned row outside the range in index order", () => { + expect(withPinnedIndex([4, 5, 6], 300)).toEqual([4, 5, 6, 300]); + expect(withPinnedIndex([4, 5, 6], 0)).toEqual([0, 4, 5, 6]); + expect(withPinnedIndex([9, 10, 11], 2)).toEqual([2, 9, 10, 11]); + }); +}); diff --git a/src/components/ui/VirtualList.tsx b/src/components/ui/VirtualList.tsx index 190ea8b..d93adb8 100644 --- a/src/components/ui/VirtualList.tsx +++ b/src/components/ui/VirtualList.tsx @@ -1,4 +1,9 @@ -import { useVirtualizer, type ScrollToOptions, type VirtualItem } from "@tanstack/react-virtual"; +import { + defaultRangeExtractor, + useVirtualizer, + type ScrollToOptions, + type VirtualItem, +} from "@tanstack/react-virtual"; import { Slot } from "radix-ui"; import { createContext, @@ -39,6 +44,7 @@ interface VirtualListProps extends Omit, "v getItemKey?: (item: T, index: number) => Key; initialViewportHeight?: number; overscan?: number; + pinnedIndex?: number; virtualListRef?: Ref; } @@ -53,11 +59,16 @@ function VirtualList({ getItemKey, initialViewportHeight = estimateHeight * 16, overscan = 8, + pinnedIndex, virtualListRef, children, ...props }: VirtualListProps) { const [viewportElement, setViewportElement] = useState(null); + const renderedPinnedIndex = + pinnedIndex !== undefined && pinnedIndex >= 0 && pinnedIndex < items.length + ? pinnedIndex + : undefined; const virtualizer = useVirtualizer({ count: items.length, @@ -69,6 +80,7 @@ function VirtualList({ width: 0, }, overscan, + rangeExtractor: (range) => withPinnedIndex(defaultRangeExtractor(range), renderedPinnedIndex), }); useImperativeHandle( @@ -113,6 +125,13 @@ function VirtualList({ ); } +// Unmounting the row that holds focus drops focus to the document body, and takes +// the collection out of the tab sequence when that row is its only tab stop. +const withPinnedIndex = (indexes: number[], pinnedIndex: number | undefined) => + pinnedIndex === undefined || indexes.includes(pinnedIndex) + ? indexes + : [...indexes, pinnedIndex].sort((left, right) => left - right); + function VirtualListContent({ asChild = false, className, @@ -192,5 +211,6 @@ export { VirtualListEmpty, VirtualListItem, VirtualListItems, + withPinnedIndex, type VirtualItem, }; diff --git a/src/features/folder-context/components/ArticleNavigator.test.tsx b/src/features/folder-context/components/ArticleNavigator.test.tsx index a82d94b..c93d7ff 100644 --- a/src/features/folder-context/components/ArticleNavigator.test.tsx +++ b/src/features/folder-context/components/ArticleNavigator.test.tsx @@ -6,7 +6,7 @@ import { createNestedArticleTree, } from "@/test/factories/folderContext"; import { TEST_NESTED_DIRECTORY_PATH } from "@/test/fixtures/paths"; -import { render, renderWithUser, screen } from "@/test/utils/react"; +import { act, render, renderWithUser, screen } from "@/test/utils/react"; import { useArticleNavigatorStore } from "../stores/articleNavigator"; import { ArticleNavigator } from "./ArticleNavigator"; @@ -157,6 +157,98 @@ describe("ArticleNavigator", () => { expect(screen.queryByRole("treeitem", { current: "page" })).not.toBeInTheDocument(); }); + it("holds a single tab stop that follows the focused row", async () => { + const { user } = renderWithUser( + , + ); + + expect(screen.getAllByRole("treeitem").map((row) => row.tabIndex)).toEqual([0, -1, -1, -1]); + + screen.getByRole("treeitem", { name: "readme.md" }).focus(); + await user.keyboard("{ArrowDown}"); + + expect(screen.getAllByRole("treeitem").map((row) => row.tabIndex)).toEqual([-1, 0, -1, -1]); + }); + + it("moves focus with the arrow keys without opening a document", async () => { + const onOpenArticle = vi.fn(); + const { user } = renderWithUser( + , + ); + + screen.getByRole("treeitem", { name: "readme.md" }).focus(); + await user.keyboard("{ArrowDown}"); + + expect(screen.getByRole("treeitem", { name: "draft.markdown" })).toHaveFocus(); + + await user.keyboard("{End}"); + + expect(screen.getByRole("treeitem", { name: "empty" })).toHaveFocus(); + + await user.keyboard("{ArrowUp}{Home}"); + + expect(screen.getByRole("treeitem", { name: "readme.md" })).toHaveFocus(); + expect(onOpenArticle).not.toHaveBeenCalled(); + }); + + it("expands, descends, and collapses a directory with the horizontal arrows", async () => { + const { user } = renderWithUser( + , + ); + + const directory = screen.getByRole("treeitem", { name: "docs" }); + directory.focus(); + await user.keyboard("{ArrowRight}"); + + expect(directory).toHaveAttribute("aria-expanded", "true"); + expect(directory).toHaveFocus(); + + await user.keyboard("{ArrowRight}"); + + expect(screen.getByRole("treeitem", { name: "spec.md" })).toHaveFocus(); + + await user.keyboard("{ArrowLeft}"); + + expect(directory).toHaveFocus(); + + await user.keyboard("{ArrowLeft}"); + + expect(directory).toHaveAttribute("aria-expanded", "false"); + expect(screen.queryByRole("treeitem", { name: "spec.md" })).not.toBeInTheDocument(); + }); + + it("keeps the tab stop on the nearest surviving row when a directory collapses", async () => { + const { user } = renderWithUser( + , + ); + + screen.getByRole("treeitem", { name: "docs" }).focus(); + await user.keyboard("{ArrowRight}{ArrowRight}"); + + expect(screen.getByRole("treeitem", { name: "spec.md" })).toHaveFocus(); + + act(() => useArticleNavigatorStore.getState().toggleDirectory(TEST_NESTED_DIRECTORY_PATH)); + + expect(screen.getByRole("treeitem", { name: "docs" }).tabIndex).toBe(0); + expect(screen.getAllByRole("treeitem").filter((row) => row.tabIndex === 0)).toHaveLength(1); + }); + it("shows when the active document is outside the current folder context", () => { render( (null); + const pendingFocusPathRef = useRef(null); + const rowElementsRef = useRef(new Map()); + const focusedIndex = getArticleNavigatorFocusedIndex(rows, focusedPath); + + useEffect(() => { + const pendingFocusPath = pendingFocusPathRef.current; + + if (pendingFocusPath === null) { + return; + } + + pendingFocusPathRef.current = null; + rowElementsRef.current.get(pendingFocusPath)?.focus(); + }); + + const focusRow = (index: number) => { + const path = rows[index]?.path; + + if (path === undefined) { + return; + } + + // The row may sit outside the rendered window, so it is pinned first and + // focused once that render commits. + pendingFocusPathRef.current = path; + setFocusedPath(path); + }; + + const activateRow = (index: number) => { + const row = rows[index]; + + if (!row) { + return; + } + + if (row.kind === "directory") { + onToggleDirectory(row.path); + } else { + onOpenArticle(row.path); + } + }; + + const handleKeyDown = (event: KeyboardEvent) => { + if (!isArticleNavigatorTraversalKey(event.key) || !hasNoShortcutModifier(event.nativeEvent)) { + return; + } + + event.preventDefault(); + + const action = getArticleNavigatorTraversalAction({ focusedIndex, key: event.key, rows }); + + switch (action?.type) { + case "activateRow": + activateRow(action.index); + break; + case "focusRow": + focusRow(action.index); + break; + case "toggleDirectory": + onToggleDirectory(action.path); + break; + } + }; + return ( row.path} items={rows} + pinnedIndex={focusedIndex} virtualListRef={virtualListRef} > - + > - {(row, virtualRow) => ( + {(row, virtualRow, index) => ( activateRow(index)} + onFocus={() => setFocusedPath(row.path)} + registerElement={(element) => + registerRowElement(rowElementsRef.current, row, element) + } row={row} virtualRow={virtualRow} /> @@ -229,31 +309,35 @@ function ArticleNavigatorRows({ ); } +const registerRowElement = ( + rowElements: Map, + row: ArticleNavigatorRow, + element: HTMLLIElement | null, +) => { + if (element) { + rowElements.set(row.path, element); + } else { + rowElements.delete(row.path); + } +}; + interface ArticleNavigatorTreeItemProps { - onOpenArticle: (path: string) => void; - onToggleDirectory: (path: string) => void; + isTabStop: boolean; + onActivate: () => void; + onFocus: () => void; + registerElement: (element: HTMLLIElement | null) => void; row: ArticleNavigatorRow; virtualRow: VirtualItem; } function ArticleNavigatorTreeItem({ - onOpenArticle, - onToggleDirectory, + isTabStop, + onActivate, + onFocus, + registerElement, row, virtualRow, }: ArticleNavigatorTreeItemProps) { - const activate = () => - row.kind === "directory" ? onToggleDirectory(row.path) : onOpenArticle(row.path); - - const handleKeyDown = (event: KeyboardEvent) => { - if ((event.key !== "Enter" && event.key !== " ") || !hasNoShortcutModifier(event.nativeEvent)) { - return; - } - - event.preventDefault(); - activate(); - }; - return ( diff --git a/src/features/folder-context/utils/articleNavigatorTraversal.test.ts b/src/features/folder-context/utils/articleNavigatorTraversal.test.ts new file mode 100644 index 0000000..58752b6 --- /dev/null +++ b/src/features/folder-context/utils/articleNavigatorTraversal.test.ts @@ -0,0 +1,112 @@ +import { describe, expect, it } from "vitest"; + +import { createNestedArticleTree } from "@/test/factories/folderContext"; +import { TEST_NESTED_DIRECTORY_PATH } from "@/test/fixtures/paths"; + +import { buildArticleNavigatorRows } from "./articleNavigatorRows"; +import { + getArticleNavigatorFocusedIndex, + getArticleNavigatorTraversalAction, + isArticleNavigatorTraversalKey, +} from "./articleNavigatorTraversal"; + +const tree = createNestedArticleTree(); + +// readme.md, draft.markdown, docs, spec.md, empty +const expandedRows = buildArticleNavigatorRows({ + activeArticlePath: null, + expandedDirectoryPaths: [TEST_NESTED_DIRECTORY_PATH], + tree, +}); + +// readme.md, draft.markdown, docs, empty +const collapsedRows = buildArticleNavigatorRows({ + activeArticlePath: null, + expandedDirectoryPaths: [], + tree, +}); + +const actionFor = (key: string, focusedIndex: number, rows = expandedRows) => + getArticleNavigatorTraversalAction({ focusedIndex, key, rows }); + +describe("article navigator traversal", () => { + it("claims only the keys the tree operates on", () => { + const traversalKeys = [ + " ", + "ArrowDown", + "ArrowLeft", + "ArrowRight", + "ArrowUp", + "End", + "Enter", + "Home", + ]; + + expect(traversalKeys.filter(isArticleNavigatorTraversalKey)).toEqual(traversalKeys); + expect(isArticleNavigatorTraversalKey("a")).toBe(false); + expect(isArticleNavigatorTraversalKey("Tab")).toBe(false); + }); + + it("moves focus one row at a time and stops at both ends", () => { + expect(actionFor("ArrowDown", 0)).toEqual({ type: "focusRow", index: 1 }); + expect(actionFor("ArrowUp", 1)).toEqual({ type: "focusRow", index: 0 }); + expect(actionFor("ArrowUp", 0)).toBeNull(); + expect(actionFor("ArrowDown", expandedRows.length - 1)).toBeNull(); + }); + + it("jumps to the first and last row", () => { + expect(actionFor("Home", 3)).toEqual({ type: "focusRow", index: 0 }); + expect(actionFor("End", 0)).toEqual({ type: "focusRow", index: expandedRows.length - 1 }); + }); + + it("expands a collapsed directory before descending into it", () => { + expect(actionFor("ArrowRight", 2, collapsedRows)).toEqual({ + type: "toggleDirectory", + path: TEST_NESTED_DIRECTORY_PATH, + }); + expect(actionFor("ArrowRight", 2)).toEqual({ type: "focusRow", index: 3 }); + }); + + it("collapses an expanded directory before leaving it", () => { + expect(actionFor("ArrowLeft", 2)).toEqual({ + type: "toggleDirectory", + path: TEST_NESTED_DIRECTORY_PATH, + }); + expect(actionFor("ArrowLeft", 3)).toEqual({ type: "focusRow", index: 2 }); + }); + + it("leaves rows with nowhere to go alone", () => { + expect(actionFor("ArrowRight", 0)).toBeNull(); + expect(actionFor("ArrowRight", 4)).toBeNull(); + expect(actionFor("ArrowLeft", 0)).toBeNull(); + expect(actionFor("Escape", 0)).toBeNull(); + expect(actionFor("ArrowDown", 0, [])).toBeNull(); + }); + + it("activates the focused row on enter and space", () => { + expect(actionFor("Enter", 3)).toEqual({ type: "activateRow", index: 3 }); + expect(actionFor(" ", 2)).toEqual({ type: "activateRow", index: 2 }); + }); + + it("starts on the open document and otherwise on the first row", () => { + const rowsWithActiveArticle = buildArticleNavigatorRows({ + activeArticlePath: `${TEST_NESTED_DIRECTORY_PATH}/spec.md`, + expandedDirectoryPaths: [TEST_NESTED_DIRECTORY_PATH], + tree, + }); + + expect(getArticleNavigatorFocusedIndex(rowsWithActiveArticle, null)).toBe(3); + expect(getArticleNavigatorFocusedIndex(expandedRows, null)).toBe(0); + }); + + it("follows a focused row by path across rebuilds", () => { + expect(getArticleNavigatorFocusedIndex(expandedRows, "c:\\notes\\docs\\spec.md")).toBe(3); + }); + + it("falls back to the deepest surviving ancestor of a row that is gone", () => { + expect( + getArticleNavigatorFocusedIndex(collapsedRows, `${TEST_NESTED_DIRECTORY_PATH}/spec.md`), + ).toBe(2); + expect(getArticleNavigatorFocusedIndex(collapsedRows, "C:/Elsewhere/other.md")).toBe(0); + }); +}); diff --git a/src/features/folder-context/utils/articleNavigatorTraversal.ts b/src/features/folder-context/utils/articleNavigatorTraversal.ts new file mode 100644 index 0000000..a9e5094 --- /dev/null +++ b/src/features/folder-context/utils/articleNavigatorTraversal.ts @@ -0,0 +1,102 @@ +import { isSameOrParentPath, isSamePath } from "@/lib/path"; + +import type { ArticleNavigatorRow } from "./articleNavigatorRows"; + +export type ArticleNavigatorTraversalAction = + | { type: "activateRow"; index: number } + | { type: "focusRow"; index: number } + | { type: "toggleDirectory"; path: string }; + +const TRAVERSAL_KEYS = new Set([ + " ", + "ArrowDown", + "ArrowLeft", + "ArrowRight", + "ArrowUp", + "End", + "Enter", + "Home", +]); + +export const isArticleNavigatorTraversalKey = (key: string) => TRAVERSAL_KEYS.has(key); + +interface GetArticleNavigatorTraversalActionOptions { + focusedIndex: number; + key: string; + rows: ArticleNavigatorRow[]; +} + +export const getArticleNavigatorTraversalAction = ({ + focusedIndex, + key, + rows, +}: GetArticleNavigatorTraversalActionOptions): ArticleNavigatorTraversalAction | null => { + const row = rows[focusedIndex]; + + if (!row) { + return null; + } + + const isExpandableDirectory = row.kind === "directory" && row.hasChildren; + + switch (key) { + case " ": + case "Enter": + return { type: "activateRow", index: focusedIndex }; + case "ArrowDown": + return focusRowAt(focusedIndex + 1, rows); + case "ArrowUp": + return focusRowAt(focusedIndex - 1, rows); + case "Home": + return focusRowAt(0, rows); + case "End": + return focusRowAt(rows.length - 1, rows); + case "ArrowRight": + if (!isExpandableDirectory) { + return null; + } + + return row.isExpanded + ? focusRowAt(focusedIndex + 1, rows) + : { type: "toggleDirectory", path: row.path }; + case "ArrowLeft": + if (isExpandableDirectory && row.isExpanded) { + return { type: "toggleDirectory", path: row.path }; + } + + return row.parentIndex === null ? null : { type: "focusRow", index: row.parentIndex }; + default: + return null; + } +}; + +export const getArticleNavigatorFocusedIndex = ( + rows: ArticleNavigatorRow[], + focusedPath: string | null, +) => { + if (focusedPath === null) { + return Math.max( + rows.findIndex((row) => row.kind === "file" && row.isActive), + 0, + ); + } + + const focusedIndex = rows.findIndex((row) => isSamePath(row.path, focusedPath)); + + if (focusedIndex >= 0) { + return focusedIndex; + } + + // A collapsed directory takes its descendants with it, so focus falls back to + // the deepest ancestor that survived. + return Math.max( + rows.findLastIndex((row) => isSameOrParentPath(row.path, focusedPath)), + 0, + ); +}; + +const focusRowAt = ( + index: number, + rows: ArticleNavigatorRow[], +): ArticleNavigatorTraversalAction | null => + index >= 0 && index < rows.length ? { type: "focusRow", index } : null; From 65c4356cd0f562a57144b614a35a3542b903ff12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ademir=20Jos=C3=A9=20Ferreira=20J=C3=BAnior?= Date: Sun, 2 Aug 2026 02:58:51 -0300 Subject: [PATCH 4/5] Jump to a navigator row by typing its name `Space` extends a running search rather than opening a document, because a space can appear in a file name. The search expires by elapsed time rather than on a timer: nothing reads the buffer between keystrokes, so a timer would only add a lifecycle to unwind on unmount. --- CHANGELOG.md | 1 + docs/specification.md | 3 +- .../components/ArticleNavigator.test.tsx | 71 ++++++++++++++++++- .../components/ArticleNavigator.tsx | 35 ++++++++- .../utils/articleNavigatorTraversal.test.ts | 35 +++++++++ .../utils/articleNavigatorTraversal.ts | 39 ++++++++++ 6 files changed, 180 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a08513f..fa03cf8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ Leafdown uses lightweight [Keep a Changelog](https://keepachangelog.com/en/1.1.0 ### Fixed - Traverse the article navigator with the arrow keys, `Home`, and `End`, and pass it with a single `Tab` instead of one per article. +- Jump to an article by typing the start of its name while the navigator has focus. - Announce the article navigator as a tree, with the nesting depth, sibling position, and expanded state of every row. - Keep empty folders in the article navigator reachable instead of skipping them. - Open the editor context popup with `Shift+F10` or the `Menu` key and operate every command in it from the keyboard. diff --git a/docs/specification.md b/docs/specification.md index 61aedaa..0ba427f 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -58,7 +58,8 @@ The article navigator is a tree and takes a single tab stop. Focus enters on the - `ArrowRight`: Expand the focused directory, or move into it when it is already expanded. - `ArrowLeft`: Collapse the focused directory, or move to the parent directory when it is already collapsed. - `Home` and `End`: Move to the first or last visible row. -- `Enter` and `Space`: Open the focused article, or expand and collapse the focused directory. Clicking a row does the same. +- Printable characters: Move to the next visible row whose name starts with what was typed, wrapping around. The search clears after a short pause, and one character repeated cycles through the rows that start with it. +- `Enter` and `Space`: Open the focused article, or expand and collapse the focused directory. Clicking a row does the same. `Space` extends a running search instead, since a space can appear in a file name. Moving focus never opens a document, so the focused row and the open document are routinely different rows. An empty directory is an ordinary row that can be focused and read, with nothing to expand. Collapsing a directory that contains the focused row moves focus to the nearest row that survives. diff --git a/src/features/folder-context/components/ArticleNavigator.test.tsx b/src/features/folder-context/components/ArticleNavigator.test.tsx index c93d7ff..8fd11f9 100644 --- a/src/features/folder-context/components/ArticleNavigator.test.tsx +++ b/src/features/folder-context/components/ArticleNavigator.test.tsx @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { createEmptyFolderContext, @@ -6,9 +6,10 @@ import { createNestedArticleTree, } from "@/test/factories/folderContext"; import { TEST_NESTED_DIRECTORY_PATH } from "@/test/fixtures/paths"; -import { act, render, renderWithUser, screen } from "@/test/utils/react"; +import { act, render, renderWithUser, screen, setupUser } from "@/test/utils/react"; import { useArticleNavigatorStore } from "../stores/articleNavigator"; +import { ARTICLE_NAVIGATOR_TYPEAHEAD_RESET_MS } from "../utils/articleNavigatorTraversal"; import { ArticleNavigator } from "./ArticleNavigator"; const folderContext = createFolderContext(); @@ -249,6 +250,72 @@ describe("ArticleNavigator", () => { expect(screen.getAllByRole("treeitem").filter((row) => row.tabIndex === 0)).toHaveLength(1); }); + describe("typeahead", () => { + // Only the clock is faked: user-event's own waits still need real timers. + beforeEach(() => vi.useFakeTimers({ toFake: ["Date"] })); + afterEach(() => vi.useRealTimers()); + + it("jumps to a row by name and forgets the search after a pause", async () => { + const user = setupUser({ delay: null }); + render( + , + ); + + screen.getByRole("treeitem", { name: "readme.md" }).focus(); + await user.keyboard("d"); + + expect(screen.getByRole("treeitem", { name: "draft.markdown" })).toHaveFocus(); + + await user.keyboard("o"); + + expect(screen.getByRole("treeitem", { name: "docs" })).toHaveFocus(); + + vi.setSystemTime(Date.now() + ARTICLE_NAVIGATOR_TYPEAHEAD_RESET_MS + 1); + await user.keyboard("d"); + + expect(screen.getByRole("treeitem", { name: "draft.markdown" })).toHaveFocus(); + }); + + it("does not open a document while searching", async () => { + const onOpenArticle = vi.fn(); + const user = setupUser({ delay: null }); + render( + , + ); + + screen.getByRole("treeitem", { name: "readme.md" }).focus(); + await user.keyboard("dra t"); + + expect(screen.getByRole("treeitem", { name: "draft.markdown" })).toHaveFocus(); + expect(onOpenArticle).not.toHaveBeenCalled(); + }); + + it("opens the focused article on space when no search is running", async () => { + const onOpenArticle = vi.fn(); + const user = setupUser({ delay: null }); + render( + , + ); + + screen.getByRole("treeitem", { name: "readme.md" }).focus(); + await user.keyboard(" "); + + expect(onOpenArticle).toHaveBeenCalledWith("C:/Notes/readme.md"); + }); + }); + it("shows when the active document is outside the current folder context", () => { render( (null); const pendingFocusPathRef = useRef(null); const rowElementsRef = useRef(new Map()); + const typeaheadRef = useRef({ buffer: "", lastKeyAtMs: 0 }); const focusedIndex = getArticleNavigatorFocusedIndex(rows, focusedPath); useEffect(() => { @@ -252,8 +256,37 @@ function ArticleNavigatorRows({ } }; + // Nothing reads the buffer between keystrokes, so it expires by elapsed time + // rather than on a timer. + const readTypeaheadBuffer = () => + Date.now() - typeaheadRef.current.lastKeyAtMs > ARTICLE_NAVIGATOR_TYPEAHEAD_RESET_MS + ? "" + : typeaheadRef.current.buffer; + + const searchByTypeahead = (character: string) => { + const typeaheadBuffer = readTypeaheadBuffer() + character; + typeaheadRef.current = { buffer: typeaheadBuffer, lastKeyAtMs: Date.now() }; + + const matchIndex = getArticleNavigatorTypeaheadIndex({ focusedIndex, rows, typeaheadBuffer }); + + if (matchIndex !== null) { + focusRow(matchIndex); + } + }; + const handleKeyDown = (event: KeyboardEvent) => { - if (!isArticleNavigatorTraversalKey(event.key) || !hasNoShortcutModifier(event.nativeEvent)) { + if (!hasNoShortcutModifier(event.nativeEvent)) { + return; + } + + if (isArticleNavigatorTypeaheadKey(event.key, readTypeaheadBuffer())) { + event.preventDefault(); + searchByTypeahead(event.key); + + return; + } + + if (!isArticleNavigatorTraversalKey(event.key)) { return; } diff --git a/src/features/folder-context/utils/articleNavigatorTraversal.test.ts b/src/features/folder-context/utils/articleNavigatorTraversal.test.ts index 58752b6..edec9d4 100644 --- a/src/features/folder-context/utils/articleNavigatorTraversal.test.ts +++ b/src/features/folder-context/utils/articleNavigatorTraversal.test.ts @@ -7,7 +7,9 @@ import { buildArticleNavigatorRows } from "./articleNavigatorRows"; import { getArticleNavigatorFocusedIndex, getArticleNavigatorTraversalAction, + getArticleNavigatorTypeaheadIndex, isArticleNavigatorTraversalKey, + isArticleNavigatorTypeaheadKey, } from "./articleNavigatorTraversal"; const tree = createNestedArticleTree(); @@ -29,6 +31,9 @@ const collapsedRows = buildArticleNavigatorRows({ const actionFor = (key: string, focusedIndex: number, rows = expandedRows) => getArticleNavigatorTraversalAction({ focusedIndex, key, rows }); +const typeaheadIndexFor = (typeaheadBuffer: string, focusedIndex: number) => + getArticleNavigatorTypeaheadIndex({ focusedIndex, rows: expandedRows, typeaheadBuffer }); + describe("article navigator traversal", () => { it("claims only the keys the tree operates on", () => { const traversalKeys = [ @@ -88,6 +93,36 @@ describe("article navigator traversal", () => { expect(actionFor(" ", 2)).toEqual({ type: "activateRow", index: 2 }); }); + it("takes printable keys as a search, and space only while one is running", () => { + expect(isArticleNavigatorTypeaheadKey("d", "")).toBe(true); + expect(isArticleNavigatorTypeaheadKey("2", "")).toBe(true); + expect(isArticleNavigatorTypeaheadKey(" ", "")).toBe(false); + expect(isArticleNavigatorTypeaheadKey(" ", "my")).toBe(true); + expect(isArticleNavigatorTypeaheadKey("ArrowDown", "")).toBe(false); + }); + + it("jumps to the next row whose name starts with the search", () => { + expect(typeaheadIndexFor("d", 0)).toBe(1); + expect(typeaheadIndexFor("DR", 0)).toBe(1); + expect(typeaheadIndexFor("e", 0)).toBe(4); + }); + + it("keeps a growing search on the row it already matched", () => { + expect(typeaheadIndexFor("dr", 1)).toBe(1); + expect(typeaheadIndexFor("d", 1)).toBe(2); + }); + + it("cycles through the rows sharing a first character when it repeats", () => { + expect(typeaheadIndexFor("d", 1)).toBe(2); + expect(typeaheadIndexFor("dd", 2)).toBe(1); + }); + + it("wraps around and reports no match", () => { + expect(typeaheadIndexFor("r", 3)).toBe(0); + expect(typeaheadIndexFor("z", 0)).toBeNull(); + expect(typeaheadIndexFor("", 0)).toBeNull(); + }); + it("starts on the open document and otherwise on the first row", () => { const rowsWithActiveArticle = buildArticleNavigatorRows({ activeArticlePath: `${TEST_NESTED_DIRECTORY_PATH}/spec.md`, diff --git a/src/features/folder-context/utils/articleNavigatorTraversal.ts b/src/features/folder-context/utils/articleNavigatorTraversal.ts index a9e5094..a936ee9 100644 --- a/src/features/folder-context/utils/articleNavigatorTraversal.ts +++ b/src/features/folder-context/utils/articleNavigatorTraversal.ts @@ -70,6 +70,45 @@ export const getArticleNavigatorTraversalAction = ({ } }; +export const ARTICLE_NAVIGATOR_TYPEAHEAD_RESET_MS = 1000; + +// Space activates the focused row, except mid-search, where it is an ordinary +// character in a file name. +export const isArticleNavigatorTypeaheadKey = (key: string, typeaheadBuffer: string) => + key.length === 1 && (key !== " " || typeaheadBuffer !== ""); + +interface GetArticleNavigatorTypeaheadIndexOptions { + focusedIndex: number; + rows: ArticleNavigatorRow[]; + typeaheadBuffer: string; +} + +export const getArticleNavigatorTypeaheadIndex = ({ + focusedIndex, + rows, + typeaheadBuffer, +}: GetArticleNavigatorTypeaheadIndexOptions) => { + // A repeated character cycles through the rows starting with it, rather than + // searching for a name made of it. + const query = ( + isRepeatedCharacter(typeaheadBuffer) ? typeaheadBuffer[0] : typeaheadBuffer + ).toLowerCase(); + + if (!query) { + return null; + } + + const searchOrder = rows + .map((_, offset) => (focusedIndex + offset) % rows.length) + // A single character always advances, so the row already focused cannot answer it. + .filter((index) => query.length > 1 || index !== focusedIndex); + + return searchOrder.find((index) => rows[index].name.toLowerCase().startsWith(query)) ?? null; +}; + +const isRepeatedCharacter = (value: string) => + value.length > 1 && value === value[0].repeat(value.length); + export const getArticleNavigatorFocusedIndex = ( rows: ArticleNavigatorRow[], focusedPath: string | null, From d3dcb8081087c9089afbc82449e249e285975862 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ademir=20Jos=C3=A9=20Ferreira=20J=C3=BAnior?= Date: Sun, 2 Aug 2026 03:07:17 -0300 Subject: [PATCH 5/5] Land focus on the row revealed in the sidebar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The revealed row is pinned alongside the focused one so it is mounted by the time the reveal reaches for it, and the request id is recorded once handled — expanding a directory renumbers the revealed row and re-runs the effect, which must not pull focus back a second time. --- CHANGELOG.md | 1 + docs/specification.md | 2 + src/components/layout/Shell.test.tsx | 17 +++++ src/components/ui/VirtualList.test.tsx | 22 +++--- src/components/ui/VirtualList.tsx | 26 ++++--- .../components/ArticleNavigator.test.tsx | 48 +++++++++++++ .../components/ArticleNavigator.tsx | 70 ++++++++++--------- 7 files changed, 133 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa03cf8..dee8fb7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ Leafdown uses lightweight [Keep a Changelog](https://keepachangelog.com/en/1.1.0 - Traverse the article navigator with the arrow keys, `Home`, and `End`, and pass it with a single `Tab` instead of one per article. - Jump to an article by typing the start of its name while the navigator has focus. +- Leave focus on the revealed row after `Reveal in sidebar`, instead of scrolling to it and leaving focus behind. - Announce the article navigator as a tree, with the nesting depth, sibling position, and expanded state of every row. - Keep empty folders in the article navigator reachable instead of skipping them. - Open the editor context popup with `Shift+F10` or the `Menu` key and operate every command in it from the keyboard. diff --git a/docs/specification.md b/docs/specification.md index 0ba427f..d81c414 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -63,6 +63,8 @@ The article navigator is a tree and takes a single tab stop. Focus enters on the Moving focus never opens a document, so the focused row and the open document are routinely different rows. An empty directory is an ordinary row that can be focused and read, with nothing to expand. Collapsing a directory that contains the focused row moves focus to the nearest row that survives. +`Reveal in sidebar` expands the ancestors of the active document, scrolls its row into view, and leaves focus on that row. + ## State Model These state axes compose. A document session, for example, can have a folder context and be saved and clean or dirty. diff --git a/src/components/layout/Shell.test.tsx b/src/components/layout/Shell.test.tsx index 1c5b652..4367a8d 100644 --- a/src/components/layout/Shell.test.tsx +++ b/src/components/layout/Shell.test.tsx @@ -117,6 +117,23 @@ describe("Shell", () => { expect(screen.getByTestId("active-document-host")).toHaveTextContent("# Spec"); }); + it("leaves focus on the revealed row after revealing from the File menu", async () => { + setDefaultSession({ + folderContext: nestedFolderContext, + activeDocument: createSavedDocument({ + path: SPEC_MARKDOWN_PATH, + content: "# Spec", + }), + }); + + const { user } = renderWithUser(); + + await user.click(screen.getByRole("menuitem", { name: "File" })); + await user.click(await screen.findByRole("menuitem", { name: /^Reveal in sidebar/u })); + + expect(screen.getByRole("treeitem", { name: "spec.md" })).toHaveFocus(); + }); + it("shows the empty folder state while preserving empty directories", () => { setDefaultSession({ folderContext: emptyFolderContext, diff --git a/src/components/ui/VirtualList.test.tsx b/src/components/ui/VirtualList.test.tsx index f69f320..1f20a7c 100644 --- a/src/components/ui/VirtualList.test.tsx +++ b/src/components/ui/VirtualList.test.tsx @@ -1,23 +1,27 @@ import { describe, expect, it } from "vitest"; -import { withPinnedIndex } from "./VirtualList"; +import { withPinnedIndexes } from "./VirtualList"; -describe("withPinnedIndex", () => { +describe("withPinnedIndexes", () => { it("leaves the rendered range alone when nothing is pinned", () => { const indexes = [4, 5, 6]; - expect(withPinnedIndex(indexes, undefined)).toBe(indexes); + expect(withPinnedIndexes(indexes, [])).toBe(indexes); }); - it("leaves the rendered range alone when the pinned row is already in it", () => { + it("leaves the rendered range alone when every pinned row is already in it", () => { const indexes = [4, 5, 6]; - expect(withPinnedIndex(indexes, 5)).toBe(indexes); + expect(withPinnedIndexes(indexes, [5, 6])).toBe(indexes); }); - it("adds a pinned row outside the range in index order", () => { - expect(withPinnedIndex([4, 5, 6], 300)).toEqual([4, 5, 6, 300]); - expect(withPinnedIndex([4, 5, 6], 0)).toEqual([0, 4, 5, 6]); - expect(withPinnedIndex([9, 10, 11], 2)).toEqual([2, 9, 10, 11]); + it("adds pinned rows outside the range in index order", () => { + expect(withPinnedIndexes([4, 5, 6], [300])).toEqual([4, 5, 6, 300]); + expect(withPinnedIndexes([4, 5, 6], [0])).toEqual([0, 4, 5, 6]); + expect(withPinnedIndexes([9, 10, 11], [2, 40])).toEqual([2, 9, 10, 11, 40]); + }); + + it("adds a row pinned twice only once", () => { + expect(withPinnedIndexes([4, 5, 6], [40, 40])).toEqual([4, 5, 6, 40]); }); }); diff --git a/src/components/ui/VirtualList.tsx b/src/components/ui/VirtualList.tsx index d93adb8..307e233 100644 --- a/src/components/ui/VirtualList.tsx +++ b/src/components/ui/VirtualList.tsx @@ -44,7 +44,7 @@ interface VirtualListProps extends Omit, "v getItemKey?: (item: T, index: number) => Key; initialViewportHeight?: number; overscan?: number; - pinnedIndex?: number; + pinnedIndexes?: number[]; virtualListRef?: Ref; } @@ -59,16 +59,14 @@ function VirtualList({ getItemKey, initialViewportHeight = estimateHeight * 16, overscan = 8, - pinnedIndex, + pinnedIndexes, virtualListRef, children, ...props }: VirtualListProps) { const [viewportElement, setViewportElement] = useState(null); - const renderedPinnedIndex = - pinnedIndex !== undefined && pinnedIndex >= 0 && pinnedIndex < items.length - ? pinnedIndex - : undefined; + const renderedPinnedIndexes = + pinnedIndexes?.filter((index) => index >= 0 && index < items.length) ?? []; const virtualizer = useVirtualizer({ count: items.length, @@ -80,7 +78,8 @@ function VirtualList({ width: 0, }, overscan, - rangeExtractor: (range) => withPinnedIndex(defaultRangeExtractor(range), renderedPinnedIndex), + rangeExtractor: (range) => + withPinnedIndexes(defaultRangeExtractor(range), renderedPinnedIndexes), }); useImperativeHandle( @@ -127,10 +126,15 @@ function VirtualList({ // Unmounting the row that holds focus drops focus to the document body, and takes // the collection out of the tab sequence when that row is its only tab stop. -const withPinnedIndex = (indexes: number[], pinnedIndex: number | undefined) => - pinnedIndex === undefined || indexes.includes(pinnedIndex) +const withPinnedIndexes = (indexes: number[], pinnedIndexes: number[]) => { + const missingIndexes = Array.from(new Set(pinnedIndexes)).filter( + (pinnedIndex) => !indexes.includes(pinnedIndex), + ); + + return missingIndexes.length === 0 ? indexes - : [...indexes, pinnedIndex].sort((left, right) => left - right); + : [...indexes, ...missingIndexes].sort((left, right) => left - right); +}; function VirtualListContent({ asChild = false, @@ -211,6 +215,6 @@ export { VirtualListEmpty, VirtualListItem, VirtualListItems, - withPinnedIndex, + withPinnedIndexes, type VirtualItem, }; diff --git a/src/features/folder-context/components/ArticleNavigator.test.tsx b/src/features/folder-context/components/ArticleNavigator.test.tsx index 8fd11f9..07bb5fc 100644 --- a/src/features/folder-context/components/ArticleNavigator.test.tsx +++ b/src/features/folder-context/components/ArticleNavigator.test.tsx @@ -250,6 +250,54 @@ describe("ArticleNavigator", () => { expect(screen.getAllByRole("treeitem").filter((row) => row.tabIndex === 0)).toHaveLength(1); }); + it("focuses the revealed row and hands it the tab stop", () => { + render( + , + ); + + act(() => + useArticleNavigatorStore + .getState() + .requestRevealArticle(`${TEST_NESTED_DIRECTORY_PATH}/spec.md`, [ + TEST_NESTED_DIRECTORY_PATH, + ]), + ); + + const revealedRow = screen.getByRole("treeitem", { name: "spec.md" }); + + expect(revealedRow).toHaveFocus(); + expect(revealedRow.tabIndex).toBe(0); + }); + + it("leaves focus alone when an unrelated directory expands after a reveal", async () => { + const { user } = renderWithUser( + , + ); + + act(() => + useArticleNavigatorStore + .getState() + .requestRevealArticle(`${TEST_NESTED_DIRECTORY_PATH}/spec.md`, [ + TEST_NESTED_DIRECTORY_PATH, + ]), + ); + await user.keyboard("{Home}"); + + expect(screen.getByRole("treeitem", { name: "readme.md" })).toHaveFocus(); + + act(() => useArticleNavigatorStore.getState().expandDirectories(["C:/Notes/empty"])); + + expect(screen.getByRole("treeitem", { name: "readme.md" })).toHaveFocus(); + }); + describe("typeahead", () => { // Only the clock is faked: user-event's own waits still need real timers. beforeEach(() => vi.useFakeTimers({ toFake: ["Date"] })); diff --git a/src/features/folder-context/components/ArticleNavigator.tsx b/src/features/folder-context/components/ArticleNavigator.tsx index 2170b01..977aab0 100644 --- a/src/features/folder-context/components/ArticleNavigator.tsx +++ b/src/features/folder-context/components/ArticleNavigator.tsx @@ -8,7 +8,7 @@ import { InfoIcon, TriangleAlertIcon, } from "lucide-react"; -import { useEffect, useRef, useState, type KeyboardEvent, type Ref } from "react"; +import { useEffect, useRef, useState, type KeyboardEvent } from "react"; import { buttonVariants } from "@/components/ui/Button"; import { Separator } from "@/components/ui/Separator"; @@ -58,10 +58,7 @@ export function ArticleNavigator({ }: ArticleNavigatorProps) { const expandedDirectoryPaths = useArticleNavigatorStore((state) => state.expandedDirectoryPaths); const expandDirectories = useArticleNavigatorStore((state) => state.expandDirectories); - const revealArticlePath = useArticleNavigatorStore((state) => state.revealArticlePath); - const revealRequestId = useArticleNavigatorStore((state) => state.revealRequestId); const toggleDirectory = useArticleNavigatorStore((state) => state.toggleDirectory); - const virtualListRef = useRef(null); const activeFileAncestorDirectoryPaths = folderContext && activeArticlePath ? getArticleAncestorDirectoryPaths(folderContext.tree, activeArticlePath) @@ -85,10 +82,6 @@ export function ArticleNavigator({ }) : []; const hasRows = rows.length > 0; - const revealRowIndex = rows.findIndex( - (row) => - row.kind === "file" && revealArticlePath !== null && isSamePath(row.path, revealArticlePath), - ); useEffect(() => { if (!activeFileAncestorDirectoryPathSignature) { @@ -98,14 +91,6 @@ export function ArticleNavigator({ expandDirectories(activeFileAncestorDirectoryPathSignature.split(PATH_SIGNATURE_SEPARATOR)); }, [activeFileAncestorDirectoryPathSignature, expandDirectories]); - useEffect(() => { - if (revealRequestId === 0 || revealRowIndex < 0) { - return; - } - - virtualListRef.current?.scrollToIndex(revealRowIndex, { align: "center" }); - }, [revealRequestId, revealRowIndex]); - const handleOpenArticle = (path: string) => { if (activeArticlePath && isSamePath(path, activeArticlePath)) { return; @@ -142,7 +127,6 @@ export function ArticleNavigator({ onOpenArticle={handleOpenArticle} onToggleDirectory={toggleDirectory} rows={rows} - virtualListRef={virtualListRef} /> )} {!folderContext.isEmpty && !hasRows && ( @@ -203,31 +187,54 @@ interface ArticleNavigatorRowsProps { onOpenArticle: (path: string) => void; onToggleDirectory: (path: string) => void; rows: ArticleNavigatorRow[]; - virtualListRef: Ref; +} + +interface ArticleNavigatorFocus { + path: string | null; + requestId: number; } function ArticleNavigatorRows({ onOpenArticle, onToggleDirectory, rows, - virtualListRef, }: ArticleNavigatorRowsProps) { - const [focusedPath, setFocusedPath] = useState(null); - const pendingFocusPathRef = useRef(null); + const virtualListRef = useRef(null); + const revealArticlePath = useArticleNavigatorStore((state) => state.revealArticlePath); + const revealRequestId = useArticleNavigatorStore((state) => state.revealRequestId); + const [focus, setFocus] = useState({ path: null, requestId: 0 }); const rowElementsRef = useRef(new Map()); const typeaheadRef = useRef({ buffer: "", lastKeyAtMs: 0 }); - const focusedIndex = getArticleNavigatorFocusedIndex(rows, focusedPath); + const focusedIndex = getArticleNavigatorFocusedIndex(rows, focus.path); + const handledRevealRequestIdRef = useRef(0); + const revealRowIndex = rows.findIndex( + (row) => + row.kind === "file" && revealArticlePath !== null && isSamePath(row.path, revealArticlePath), + ); + const revealRowPath = revealRowIndex < 0 ? null : rows[revealRowIndex].path; + // A requested row may sit outside the rendered window, so it is pinned first and + // focused once that render commits. useEffect(() => { - const pendingFocusPath = pendingFocusPathRef.current; + if (focus.requestId === 0 || focus.path === null) { + return; + } - if (pendingFocusPath === null) { + rowElementsRef.current.get(focus.path)?.focus(); + }, [focus]); + + // The revealed row is pinned, so it is mounted by the time this reaches for it. + useEffect(() => { + // Expanding a directory renumbers the revealed row and re-runs this, which + // must not pull focus back a second time. + if (revealRequestId === handledRevealRequestIdRef.current || revealRowPath === null) { return; } - pendingFocusPathRef.current = null; - rowElementsRef.current.get(pendingFocusPath)?.focus(); - }); + handledRevealRequestIdRef.current = revealRequestId; + virtualListRef.current?.scrollToIndex(revealRowIndex, { align: "center" }); + rowElementsRef.current.get(revealRowPath)?.focus(); + }, [revealRequestId, revealRowIndex, revealRowPath]); const focusRow = (index: number) => { const path = rows[index]?.path; @@ -236,10 +243,7 @@ function ArticleNavigatorRows({ return; } - // The row may sit outside the rendered window, so it is pinned first and - // focused once that render commits. - pendingFocusPathRef.current = path; - setFocusedPath(path); + setFocus((currentFocus) => ({ path, requestId: currentFocus.requestId + 1 })); }; const activateRow = (index: number) => { @@ -313,7 +317,7 @@ function ArticleNavigatorRows({ estimateHeight={ARTICLE_NAVIGATOR_ROW_HEIGHT} getItemKey={(row) => row.path} items={rows} - pinnedIndex={focusedIndex} + pinnedIndexes={[focusedIndex, revealRowIndex]} virtualListRef={virtualListRef} > activateRow(index)} - onFocus={() => setFocusedPath(row.path)} + onFocus={() => setFocus((currentFocus) => ({ ...currentFocus, path: row.path }))} registerElement={(element) => registerRowElement(rowElementsRef.current, row, element) }