diff --git a/frontend/src/plugins/impl/FileBrowserPlugin.tsx b/frontend/src/plugins/impl/FileBrowserPlugin.tsx index fb48c6ba329..5bc4b900386 100644 --- a/frontend/src/plugins/impl/FileBrowserPlugin.tsx +++ b/frontend/src/plugins/impl/FileBrowserPlugin.tsx @@ -1,7 +1,7 @@ /* Copyright 2026 Marimo. All rights reserved. */ import { type LucideIcon, CornerLeftUp } from "lucide-react"; -import { type JSX, useEffect, useState } from "react"; +import { type JSX, useEffect, useLayoutEffect, useRef, useState } from "react"; import { z } from "zod"; import { FILE_ICON as FILE_TYPE_ICONS, @@ -129,6 +129,7 @@ interface FileBrowserProps extends Data, PluginFunctions { } interface CheckboxOrIconProps { + name: string; isSelected: boolean; canSelect: boolean; Icon: LucideIcon; @@ -136,6 +137,7 @@ interface CheckboxOrIconProps { } function CheckboxOrIcon({ + name, isSelected, canSelect, Icon, @@ -146,17 +148,21 @@ function CheckboxOrIcon({ <> { onSelect(); e.stopPropagation(); }} - className={cn({ "hidden group-hover:flex": !isSelected })} + className={cn({ + "hidden group-hover:flex group-focus:flex": !isSelected, + })} /> @@ -165,6 +171,18 @@ function CheckboxOrIcon({ return ; } +interface RowModel { + key: string; + name: string; + Icon: LucideIcon; + isSelected: boolean; + canSelect: boolean; + /** Enter and mouse-click action. */ + onPrimary: () => void; + /** Space action; null when the row has nothing to toggle. */ + onToggleSelect: (() => void) | null; +} + /** * File browser component. * @@ -184,6 +202,13 @@ export const FileBrowser = ({ const [path, setPath] = useInternalStateWithSync(initialPath); const [isUpdatingPath, setIsUpdatingPath] = useState(false); const [showLoadingOverlay, setShowLoadingOverlay] = useState(false); + const [activeIndex, setActiveIndex] = useState(0); + const rowRefs = useRef<(HTMLTableRowElement | null)[]>([]); + const gridRef = useRef(null); + // Set when navigation is triggered from within the grid, so focus can follow + // to the parent row after the new listing renders instead of falling to the + // body when the previously focused row unmounts. + const refocusParentRef = useRef(false); // HACK: use the random-id of the host element to force a re-render // when the random-id changes, this means the cell was re-rendered @@ -210,6 +235,22 @@ export const FileBrowser = ({ }; }, [isPending]); + // Reset the roving tabindex whenever the listing reloads (a new path or a + // same-path refresh) so activeIndex never points past the current rows. + const listingKey = `${path}::${randomId}`; + const [prevListingKey, setPrevListingKey] = useState(listingKey); + if (prevListingKey !== listingKey) { + setPrevListingKey(listingKey); + setActiveIndex(0); + } + + useLayoutEffect(() => { + if (refocusParentRef.current) { + refocusParentRef.current = false; + rowRefs.current[0]?.focus(); + } + }, [listingKey]); + const files = data?.files ?? []; const selectedPaths = new Set(value.map((x) => x.path)); const canSelectDirectories = @@ -269,6 +310,8 @@ export const FileBrowser = ({ return; } + refocusParentRef.current = + gridRef.current?.contains(document.activeElement) ?? false; setPath(newPath); setIsUpdatingPath(false); } @@ -333,78 +376,90 @@ export const FileBrowser = ({ setValue([...value, ...filesInView]); } - // Create rows for directories and files - const fileRows: React.ReactNode[] = []; - - // Parent directory ".." row button - fileRows.push( - setNewPath(PARENT_DIRECTORY)} - > - - - - {PARENT_DIRECTORY} - , - ); + const rowModels: RowModel[] = [ + { + key: "parent", + name: PARENT_DIRECTORY, + Icon: CornerLeftUp, + isSelected: false, + canSelect: false, + onPrimary: () => setNewPath(PARENT_DIRECTORY), + onToggleSelect: null, + }, + ...files.map((file): RowModel => { + let filePath = file.path; + if (filePath.startsWith("//")) { + filePath = filePath.slice(1) as FilePath; + } - for (const file of files) { - let filePath = file.path; + const canSelect = + (canSelectDirectories && file.is_directory) || + (canSelectFiles && !file.is_directory); + const isSelected = selectedPaths.has(filePath); + const fileType: FileType = file.is_directory + ? "directory" + : guessFileType(file.name); + + const toggle = () => + handleSelection({ + path: filePath, + name: file.name, + isDirectory: file.is_directory, + }); + + return { + key: file.id, + name: file.name, + Icon: FILE_TYPE_ICONS[fileType], + isSelected, + canSelect, + onPrimary: file.is_directory + ? () => setNewPath(filePath) + : canSelect + ? toggle + : () => {}, + onToggleSelect: canSelect ? toggle : null, + }; + }), + ]; - if (filePath.startsWith("//")) { - filePath = filePath.slice(1) as FilePath; - } + function focusRow(index: number) { + setActiveIndex(index); + rowRefs.current[index]?.focus(); + } - // Click handler - const handleClick = file.is_directory - ? ({ path }: { path: string }) => setNewPath(path) - : handleSelection; - - // Icon - const fileType: FileType = file.is_directory - ? "directory" - : guessFileType(file.name); - - const Icon = FILE_TYPE_ICONS[fileType]; - - const isSelected = selectedPaths.has(filePath); - - fileRows.push( - - handleClick({ - path: filePath, - name: file.name, - isDirectory: file.is_directory, - }) - } - > - - - handleSelection({ - path: filePath, - name: file.name, - isDirectory: file.is_directory, - }) - } - /> - - {file.name} - , - ); + function handleRowKeyDown( + e: React.KeyboardEvent, + index: number, + ) { + const lastIndex = rowModels.length - 1; + switch (e.key) { + case "ArrowDown": + e.preventDefault(); + focusRow(Math.min(index + 1, lastIndex)); + break; + case "ArrowUp": + e.preventDefault(); + focusRow(Math.max(index - 1, 0)); + break; + case "Home": + e.preventDefault(); + focusRow(0); + break; + case "End": + e.preventDefault(); + focusRow(lastIndex); + break; + case "Enter": + e.preventDefault(); + rowModels[index].onPrimary(); + break; + // Space is select-only; preventDefault stops the list from scrolling. + case " ": + e.preventDefault(); + rowModels[index].onToggleSelect?.(); + break; + } } // Get list of parent directories. @@ -490,8 +545,44 @@ export const FileBrowser = ({ Listing files... )} - - {fileRows} +
+ + {rowModels.map((row, index) => ( + { + rowRefs.current[index] = el; + }} + tabIndex={index === activeIndex ? 0 : -1} + onFocus={() => setActiveIndex(index)} + onKeyDown={(e) => handleRowKeyDown(e, index)} + className={cn( + "hover:bg-accent group select-none focus-visible:outline-hidden focus-visible:ring-1 focus-visible:ring-ring focus-visible:ring-inset", + { "bg-primary/25 hover:bg-primary/35": row.isSelected }, + )} + aria-selected={row.canSelect ? row.isSelected : undefined} + onClick={row.onPrimary} + > + + row.onToggleSelect?.()} + /> + + {row.name} + + ))} +
diff --git a/frontend/src/plugins/impl/__tests__/FileBrowserPlugin.test.tsx b/frontend/src/plugins/impl/__tests__/FileBrowserPlugin.test.tsx new file mode 100644 index 00000000000..f16b2c21a4c --- /dev/null +++ b/frontend/src/plugins/impl/__tests__/FileBrowserPlugin.test.tsx @@ -0,0 +1,314 @@ +/* Copyright 2026 Marimo. All rights reserved. */ + +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { beforeAll, describe, expect, it, vi } from "vitest"; +import { SetupMocks } from "@/__mocks__/common"; +import { initialModeAtom } from "@/core/mode"; +import { store } from "@/core/state/jotai"; +import type { IPluginProps } from "../../types"; +import { FileBrowserPlugin } from "../FileBrowserPlugin"; + +interface MockFile { + id: string; + path: string; + name: string; + is_directory: boolean; +} + +const FILES: MockFile[] = [ + { id: "1", path: "/home/user/docs", name: "docs", is_directory: true }, + { id: "2", path: "/home/user/a.txt", name: "a.txt", is_directory: false }, + { id: "3", path: "/home/user/b.txt", name: "b.txt", is_directory: false }, +]; + +type Value = MockFile[]; + +function mockListDirectory(files: MockFile[]) { + return vi.fn().mockResolvedValue({ + files, + total_count: files.length, + is_truncated: false, + }); +} + +function makeProps( + overrides: { + selectionMode?: string; + multiple?: boolean; + value?: Value; + setValue?: (v: Value) => void; + files?: MockFile[]; + list_directory?: ReturnType; + host?: HTMLElement; + } = {}, +): IPluginProps> { + const files = overrides.files ?? FILES; + const list_directory = overrides.list_directory ?? mockListDirectory(files); + return { + data: { + initialPath: "/home/user", + filetypes: [], + selectionMode: overrides.selectionMode ?? "all", + multiple: overrides.multiple ?? true, + label: null, + restrictNavigation: false, + }, + value: overrides.value ?? [], + setValue: overrides.setValue ?? vi.fn(), + host: overrides.host ?? document.createElement("div"), + functions: { + list_directory, + }, + } as unknown as IPluginProps>; +} + +function renderBrowser(overrides: Parameters[0] = {}) { + const listDirectory = + overrides.list_directory ?? mockListDirectory(overrides.files ?? FILES); + const result = render( + FileBrowserPlugin.render( + makeProps({ ...overrides, list_directory: listDirectory }) as Parameters< + typeof FileBrowserPlugin.render + >[0], + ), + ); + return { ...result, listDirectory }; +} + +beforeAll(() => { + SetupMocks.resizeObserver(); + store.set(initialModeAtom, "edit"); +}); + +describe("FileBrowserPlugin keyboard accessibility", () => { + it("renders a row per file plus the parent row", async () => { + renderBrowser(); + expect(await screen.findByText("docs")).toBeInTheDocument(); + // parent "..", docs, a.txt, b.txt + expect(screen.getAllByRole("row")).toHaveLength(4); + }); + + it("marks the list as a multiselectable grid", async () => { + renderBrowser({ multiple: true }); + await screen.findByText("docs"); + const grid = screen.getByRole("grid"); + expect(grid).toHaveAttribute("aria-multiselectable", "true"); + }); + + it("does not select a non-selectable file on click (mode=directory)", async () => { + const setValue = vi.fn(); + renderBrowser({ selectionMode: "directory", setValue }); + const fileCell = await screen.findByText("a.txt"); + fireEvent.click(fileCell.closest('[role="row"]')!); + expect(setValue).not.toHaveBeenCalled(); + }); + + it("labels each selectable checkbox with the file name", async () => { + renderBrowser({ selectionMode: "all" }); + await screen.findByText("docs"); + expect( + screen.getByRole("checkbox", { name: "Select a.txt" }), + ).toBeInTheDocument(); + expect( + screen.getByRole("checkbox", { name: "Select docs" }), + ).toBeInTheDocument(); + }); + + it("keeps checkboxes out of the tab order", async () => { + renderBrowser({ selectionMode: "all" }); + await screen.findByText("docs"); + expect( + screen.getByRole("checkbox", { name: "Select a.txt" }), + ).toHaveAttribute("tabindex", "-1"); + }); + + it("places exactly one row in the tab order", async () => { + renderBrowser(); + await screen.findByText("docs"); + const rows = screen.getAllByRole("row"); + const tabbable = rows.filter((r) => r.getAttribute("tabindex") === "0"); + expect(tabbable).toHaveLength(1); + // the parent row is first and starts active + expect(rows[0]).toHaveAttribute("tabindex", "0"); + }); + + it("resets the active row to the parent row after navigating", async () => { + renderBrowser({ selectionMode: "all" }); + const docs = await screen.findByText("docs"); + const docsRow = docs.closest('[role="row"]')!; + fireEvent.keyDown(docsRow, { key: "ArrowDown" }); // move active off the parent + fireEvent.click(docs); // navigate into "docs" + await screen.findByText("docs"); // listing reloads (mock returns same files) + const rows = screen.getAllByRole("row"); + expect(rows[0]).toHaveAttribute("tabindex", "0"); + // the focused row must match the only tabbable row + expect(rows[0]).toHaveFocus(); + }); + + it("resets the active row when the listing refreshes in place", async () => { + // The cell's random-id changes when the cell re-renders, refetching the + // same path. A shrinking listing must not leave activeIndex out of bounds. + const parent = document.createElement("div"); + parent.setAttribute("random-id", "a"); + const host = document.createElement("div"); + parent.append(host); + document.body.append(parent); + + const list_directory = vi + .fn() + .mockResolvedValueOnce({ + files: FILES, + total_count: FILES.length, + is_truncated: false, + }) + .mockResolvedValue({ + files: [FILES[0]], + total_count: 1, + is_truncated: false, + }); + + const props = () => + FileBrowserPlugin.render( + makeProps({ host, list_directory }) as Parameters< + typeof FileBrowserPlugin.render + >[0], + ); + const { rerender } = render(props()); + await screen.findByText("a.txt"); + + // Move the active row to the last row, then trigger a same-path refresh. + fireEvent.keyDown(screen.getAllByRole("row")[0], { key: "End" }); + expect(screen.getAllByRole("row").at(-1)).toHaveAttribute("tabindex", "0"); + + parent.setAttribute("random-id", "b"); + rerender(props()); + + await waitFor(() => + // parent ".." plus the single remaining file + expect(screen.getAllByRole("row")).toHaveLength(2), + ); + const rows = screen.getAllByRole("row"); + const tabbable = rows.filter((r) => r.getAttribute("tabindex") === "0"); + expect(tabbable).toHaveLength(1); + expect(rows[0]).toHaveAttribute("tabindex", "0"); + }); + + it("syncs the tabbable row to whichever row gains focus", async () => { + renderBrowser(); + await screen.findByText("docs"); + const rows = screen.getAllByRole("row"); // [.., docs, a.txt, b.txt] + expect(rows[0]).toHaveAttribute("tabindex", "0"); + + fireEvent.focus(rows[2]); // e.g. focus from a mouse click, not arrow keys + expect(rows[2]).toHaveAttribute("tabindex", "0"); + expect(rows[0]).toHaveAttribute("tabindex", "-1"); + }); + + it("moves focus to the parent row after navigating from within the grid", async () => { + // Navigating into a directory unmounts the focused row; focus must follow + // to the parent row instead of falling back to the document body. + const list_directory = vi + .fn() + .mockResolvedValueOnce({ + files: FILES, + total_count: FILES.length, + is_truncated: false, + }) + .mockResolvedValue({ + files: [ + { + id: "99", + path: "/home/user/docs/inner.txt", + name: "inner.txt", + is_directory: false, + }, + ], + total_count: 1, + is_truncated: false, + }); + renderBrowser({ selectionMode: "all", list_directory }); + await screen.findByText("docs"); + + const rows = screen.getAllByRole("row"); // [.., docs, a.txt, b.txt] + fireEvent.keyDown(rows[0], { key: "ArrowDown" }); // focus the docs row + expect(rows[1]).toHaveFocus(); + fireEvent.keyDown(rows[1], { key: "Enter" }); // navigate into docs + + await screen.findByText("inner.txt"); + expect(screen.getAllByRole("row")[0]).toHaveFocus(); + }); + + it("moves focus with arrows and clamps at the ends", async () => { + renderBrowser(); + await screen.findByText("docs"); + const rows = screen.getAllByRole("row"); // [.., docs, a.txt, b.txt] + + fireEvent.keyDown(rows[0], { key: "ArrowDown" }); + expect(rows[1]).toHaveFocus(); + expect(rows[1]).toHaveAttribute("tabindex", "0"); + + fireEvent.keyDown(rows[1], { key: "ArrowUp" }); + expect(rows[0]).toHaveFocus(); + + fireEvent.keyDown(rows[0], { key: "ArrowUp" }); // clamp at top + expect(rows[0]).toHaveFocus(); + + fireEvent.keyDown(rows[0], { key: "End" }); + expect(rows[3]).toHaveFocus(); + + fireEvent.keyDown(rows[3], { key: "ArrowDown" }); // clamp at bottom + expect(rows[3]).toHaveFocus(); + + fireEvent.keyDown(rows[3], { key: "Home" }); + expect(rows[0]).toHaveFocus(); + }); + + it("Enter navigates into a directory", async () => { + const setValue = vi.fn(); + const { listDirectory } = renderBrowser({ + selectionMode: "all", + setValue, + }); + await screen.findByText("docs"); + fireEvent.keyDown(rowFor("docs"), { key: "Enter" }); + await waitFor(() => + expect(listDirectory).toHaveBeenCalledWith({ path: "/home/user/docs" }), + ); + // navigation does not mutate value + expect(setValue).not.toHaveBeenCalled(); + }); + + it("Enter toggles selection on a selectable file", async () => { + const setValue = vi.fn(); + renderBrowser({ selectionMode: "all", multiple: true, setValue }); + await screen.findByText("a.txt"); + fireEvent.keyDown(rowFor("a.txt"), { key: "Enter" }); + expect(setValue).toHaveBeenCalledWith([ + expect.objectContaining({ path: "/home/user/a.txt" }), + ]); + }); + + it("Space toggles selection and never navigates", async () => { + const setValue = vi.fn(); + renderBrowser({ selectionMode: "all", multiple: true, setValue }); + await screen.findByText("docs"); + // Space on a selectable directory selects it (does not navigate) + fireEvent.keyDown(rowFor("docs"), { key: " " }); + expect(setValue).toHaveBeenCalledWith([ + expect.objectContaining({ path: "/home/user/docs" }), + ]); + }); + + it("Space on the parent row is a no-op", async () => { + const setValue = vi.fn(); + renderBrowser({ selectionMode: "all", setValue }); + await screen.findByText("docs"); + const parentRow = screen.getAllByRole("row")[0]; + fireEvent.keyDown(parentRow, { key: " " }); + expect(setValue).not.toHaveBeenCalled(); + }); +}); + +function rowFor(name: string): HTMLElement { + return screen.getByText(name).closest('[role="row"]') as HTMLElement; +}