diff --git a/CHANGELOG.md b/CHANGELOG.md index 96ee8ff..dee8fb7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,11 @@ 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. +- 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. - 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..59ded58 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -249,6 +249,21 @@ - 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, 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. 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:** + +- `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. +- 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 ### Windows first, cross-platform aware diff --git a/docs/specification.md b/docs/specification.md index 1ec49b3..d81c414 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -50,6 +50,21 @@ 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. +- 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. + +`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 7f664b1..4367a8d 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,17 +106,34 @@ 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"); }); + 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, @@ -120,7 +143,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 +167,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 +185,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.test.tsx b/src/components/ui/VirtualList.test.tsx new file mode 100644 index 0000000..1f20a7c --- /dev/null +++ b/src/components/ui/VirtualList.test.tsx @@ -0,0 +1,27 @@ +import { describe, expect, it } from "vitest"; + +import { withPinnedIndexes } from "./VirtualList"; + +describe("withPinnedIndexes", () => { + it("leaves the rendered range alone when nothing is pinned", () => { + const indexes = [4, 5, 6]; + + expect(withPinnedIndexes(indexes, [])).toBe(indexes); + }); + + it("leaves the rendered range alone when every pinned row is already in it", () => { + const indexes = [4, 5, 6]; + + expect(withPinnedIndexes(indexes, [5, 6])).toBe(indexes); + }); + + 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 e8b9479..307e233 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; + pinnedIndexes?: number[]; virtualListRef?: Ref; } @@ -53,11 +59,14 @@ function VirtualList({ getItemKey, initialViewportHeight = estimateHeight * 16, overscan = 8, + pinnedIndexes, virtualListRef, children, ...props }: VirtualListProps) { const [viewportElement, setViewportElement] = useState(null); + const renderedPinnedIndexes = + pinnedIndexes?.filter((index) => index >= 0 && index < items.length) ?? []; const virtualizer = useVirtualizer({ count: items.length, @@ -69,6 +78,8 @@ function VirtualList({ width: 0, }, overscan, + rangeExtractor: (range) => + withPinnedIndexes(defaultRangeExtractor(range), renderedPinnedIndexes), }); useImperativeHandle( @@ -113,6 +124,18 @@ 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 withPinnedIndexes = (indexes: number[], pinnedIndexes: number[]) => { + const missingIndexes = Array.from(new Set(pinnedIndexes)).filter( + (pinnedIndex) => !indexes.includes(pinnedIndex), + ); + + return missingIndexes.length === 0 + ? indexes + : [...indexes, ...missingIndexes].sort((left, right) => left - right); +}; + function VirtualListContent({ asChild = false, className, @@ -186,4 +209,12 @@ function VirtualListEmpty({ children }: { children: ReactNode }) { return children; } -export { VirtualList, VirtualListContent, VirtualListEmpty, VirtualListItem, VirtualListItems }; +export { + VirtualList, + VirtualListContent, + VirtualListEmpty, + VirtualListItem, + VirtualListItems, + withPinnedIndexes, + type VirtualItem, +}; diff --git a/src/features/folder-context/components/ArticleNavigator.test.tsx b/src/features/folder-context/components/ArticleNavigator.test.tsx index cf3747b..07bb5fc 100644 --- a/src/features/folder-context/components/ArticleNavigator.test.tsx +++ b/src/features/folder-context/components/ArticleNavigator.test.tsx @@ -1,13 +1,21 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { createEmptyFolderContext, createFolderContext } from "@/test/factories/folderContext"; -import { render, renderWithUser, screen } from "@/test/utils/react"; +import { + createEmptyFolderContext, + createFolderContext, + createNestedArticleTree, +} from "@/test/factories/folderContext"; +import { TEST_NESTED_DIRECTORY_PATH } from "@/test/fixtures/paths"; +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(); +const nestedFolderContext = createFolderContext({ tree: createNestedArticleTree() }); + const emptyFolderContext = createEmptyFolderContext(); const folderContextWithScanWarning = createFolderContext({ @@ -33,7 +41,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 +72,298 @@ 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("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("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"] })); + 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( 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) @@ -75,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) { @@ -88,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; @@ -132,7 +127,6 @@ export function ArticleNavigator({ onOpenArticle={handleOpenArticle} onToggleDirectory={toggleDirectory} rows={rows} - virtualListRef={virtualListRef} /> )} {!folderContext.isEmpty && !hasRows && ( @@ -193,32 +187,158 @@ 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 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, 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(() => { + if (focus.requestId === 0 || focus.path === null) { + return; + } + + 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; + } + + 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; + + if (path === undefined) { + return; + } + + setFocus((currentFocus) => ({ path, requestId: currentFocus.requestId + 1 })); + }; + + const activateRow = (index: number) => { + const row = rows[index]; + + if (!row) { + return; + } + + if (row.kind === "directory") { + onToggleDirectory(row.path); + } else { + onOpenArticle(row.path); + } + }; + + // 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 (!hasNoShortcutModifier(event.nativeEvent)) { + return; + } + + if (isArticleNavigatorTypeaheadKey(event.key, readTypeaheadBuffer())) { + event.preventDefault(); + searchByTypeahead(event.key); + + return; + } + + if (!isArticleNavigatorTraversalKey(event.key)) { + 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} + pinnedIndexes={[focusedIndex, revealRowIndex]} virtualListRef={virtualListRef} > - + > - {(row, virtualRow) => ( - - {row.kind === "directory" && ( - - )} - {row.kind === "file" && } - + {(row, virtualRow, index) => ( + activateRow(index)} + onFocus={() => setFocus((currentFocus) => ({ ...currentFocus, path: row.path }))} + registerElement={(element) => + registerRowElement(rowElementsRef.current, row, element) + } + row={row} + virtualRow={virtualRow} + /> )} @@ -226,25 +346,71 @@ function ArticleNavigatorRows({ ); } -interface DirectoryRowProps { - row: ArticleNavigatorDirectoryRow; - onToggleDirectory: (path: string) => void; -} +const registerRowElement = ( + rowElements: Map, + row: ArticleNavigatorRow, + element: HTMLLIElement | null, +) => { + if (element) { + rowElements.set(row.path, element); + } else { + rowElements.delete(row.path); + } +}; -function DirectoryRow({ row, onToggleDirectory }: DirectoryRowProps) { - const Icon = row.isExpanded ? ChevronDownIcon : ChevronRightIcon; +interface ArticleNavigatorTreeItemProps { + isTabStop: boolean; + onActivate: () => void; + onFocus: () => void; + registerElement: (element: HTMLLIElement | null) => void; + row: ArticleNavigatorRow; + virtualRow: VirtualItem; +} +function ArticleNavigatorTreeItem({ + isTabStop, + onActivate, + onFocus, + registerElement, + row, + virtualRow, +}: ArticleNavigatorTreeItemProps) { return ( - + ); } -interface ArticleRowProps { - row: ArticleNavigatorArticleRow; - onOpenArticle: (path: string) => void; -} - -function ArticleRow({ row, onOpenArticle }: ArticleRowProps) { +function ArticleRowContent({ row }: { row: ArticleNavigatorArticleRow }) { return ( - + ); } 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 }; + }); +}; 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..edec9d4 --- /dev/null +++ b/src/features/folder-context/utils/articleNavigatorTraversal.test.ts @@ -0,0 +1,147 @@ +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, + getArticleNavigatorTypeaheadIndex, + isArticleNavigatorTraversalKey, + isArticleNavigatorTypeaheadKey, +} 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 }); + +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 = [ + " ", + "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("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`, + 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..a936ee9 --- /dev/null +++ b/src/features/folder-context/utils/articleNavigatorTraversal.ts @@ -0,0 +1,141 @@ +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 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, +) => { + 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;