diff --git a/src/renderer/src/components/sidebar/RemoteFileBrowser.keyboard.test.tsx b/src/renderer/src/components/sidebar/RemoteFileBrowser.keyboard.test.tsx new file mode 100644 index 00000000000..5205a2e0fb7 --- /dev/null +++ b/src/renderer/src/components/sidebar/RemoteFileBrowser.keyboard.test.tsx @@ -0,0 +1,220 @@ +// @vitest-environment happy-dom + +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { RemoteFileBrowser } from './RemoteFileBrowser' + +const browseDir = vi.fn(async ({ dirPath }: { dirPath: string }) => ({ + entries: [ + { name: 'docs', isDirectory: true }, + { name: 'src', isDirectory: true }, + { name: 'README.md', isDirectory: false } + ], + resolvedPath: dirPath === '~' ? '/home/alice' : dirPath, + pathFlavor: 'posix' as const +})) + +async function flushPromises(): Promise { + for (let index = 0; index < 4; index += 1) { + await Promise.resolve() + } +} + +async function render(): Promise<{ + container: HTMLDivElement + input: HTMLInputElement + root: Root +}> { + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container) + await act(async () => { + root.render() + await flushPromises() + }) + const input = container.querySelector('input') + if (!input) { + throw new Error('Remote file browser input was not rendered') + } + return { container, input, root } +} + +async function press( + input: HTMLInputElement, + key: string, + modifiers: KeyboardEventInit = {} +): Promise { + await act(async () => { + input.dispatchEvent( + new KeyboardEvent('keydown', { key, bubbles: true, cancelable: true, ...modifiers }) + ) + await flushPromises() + }) +} + +async function type(input: HTMLInputElement, value: string): Promise { + await act(async () => { + Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')?.set?.call(input, value) + input.dispatchEvent(new Event('input', { bubbles: true })) + }) +} + +async function settlePathResolve(): Promise { + await act(async () => { + vi.advanceTimersByTime(300) + await flushPromises() + }) +} + +function selectedRow(container: HTMLElement): string | null | undefined { + return container.querySelector('[role="option"][aria-selected="true"]')?.textContent +} + +describe('RemoteFileBrowser keyboard navigation', () => { + beforeEach(() => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true + browseDir.mockClear() + Object.defineProperty(window, 'api', { configurable: true, value: { ssh: { browseDir } } }) + }) + + afterEach(() => { + vi.useRealTimers() + document.body.replaceChildren() + vi.restoreAllMocks() + }) + + it('highlights the first row and moves it with the arrow keys', async () => { + const { container, input, root } = await render() + expect(selectedRow(container)).toBe('docs') + expect(input.getAttribute('aria-activedescendant')).toBe( + container.querySelector('[role="option"][aria-selected="true"]')?.id + ) + expect(input.getAttribute('role')).toBe('combobox') + expect(input.getAttribute('aria-controls')).toBe( + container.querySelector('[role="listbox"]')?.id + ) + + await press(input, 'ArrowDown') + await press(input, 'ArrowDown') + await press(input, 'ArrowDown') + expect(selectedRow(container)).toBe('README.md') + + await press(input, 'ArrowUp') + expect(selectedRow(container)).toBe('src') + + // Modified arrows stay with the input (selection, caret jumps). + await press(input, 'ArrowDown', { shiftKey: true }) + expect(selectedRow(container)).toBe('src') + + await act(async () => root.unmount()) + }) + + it('opens the highlighted folder on Enter', async () => { + const { container, input, root } = await render() + browseDir.mockClear() + + await press(input, 'ArrowDown') + await press(input, 'Enter') + + expect(browseDir).toHaveBeenCalledWith({ targetId: 'target-1', dirPath: '/home/alice/src' }) + expect(selectedRow(container)).toBe('docs') + + await act(async () => root.unmount()) + }) + + it('points the input at no row while the next listing loads', async () => { + const { input, root } = await render() + browseDir.mockImplementationOnce(() => new Promise(() => {})) + + await press(input, 'Enter') + + expect(input.hasAttribute('aria-activedescendant')).toBe(false) + await act(async () => root.unmount()) + }) + + // Why: the previous listing's single folder would otherwise win Enter while the next one loads. + it('ignores Enter while the next listing loads', async () => { + browseDir.mockImplementationOnce(async () => ({ + entries: [ + { name: 'docs', isDirectory: true }, + { name: 'README.md', isDirectory: false } + ], + resolvedPath: '/home/alice', + pathFlavor: 'posix' as const + })) + const { input, root } = await render() + browseDir.mockClear() + browseDir.mockImplementationOnce(() => new Promise(() => {})) + + await press(input, 'Enter') + await press(input, 'Enter') + + expect(browseDir).toHaveBeenCalledTimes(1) + await act(async () => root.unmount()) + }) + + // Why: the cached home listing comes back as the same array under `~/`, which must not keep the moved row. + it('commits a typed path instead of a row highlighted before typing it', async () => { + vi.useFakeTimers() + const { input, root } = await render() + await press(input, 'ArrowDown') + browseDir.mockClear() + + await type(input, '~/') + await settlePathResolve() + await press(input, 'Enter') + + // Committing `~/` reopens the cached home listing and clears the typed path. + expect(input.value).toBe('') + expect(browseDir).not.toHaveBeenCalledWith({ + targetId: 'target-1', + dirPath: '/home/alice/src' + }) + await act(async () => root.unmount()) + }) + + // Why: until the debounce resolves `docs/`, the preview and its moved highlight still show `~/`. + it('ignores Enter on a typed directory that has not resolved yet', async () => { + vi.useFakeTimers() + const { input, root } = await render() + await type(input, '~/') + await settlePathResolve() + await press(input, 'ArrowDown') + await press(input, 'ArrowDown') + browseDir.mockClear() + + await type(input, '~/docs/') + await press(input, 'Enter') + expect(browseDir).not.toHaveBeenCalled() + + await settlePathResolve() + await press(input, 'Enter') + expect(browseDir).toHaveBeenLastCalledWith({ + targetId: 'target-1', + dirPath: '/home/alice/docs' + }) + await act(async () => root.unmount()) + }) + + // Why: before the first preview, `.` still filters the old rows, so the default highlight is a dotfolder. + it('ignores Enter on path input that has no preview yet', async () => { + vi.useFakeTimers() + browseDir.mockImplementationOnce(async () => ({ + entries: [ + { name: '.config', isDirectory: true }, + { name: '.ssh', isDirectory: true } + ], + resolvedPath: '/home/alice', + pathFlavor: 'posix' as const + })) + const { input, root } = await render() + browseDir.mockClear() + + await type(input, '.') + await press(input, 'Enter') + + expect(browseDir).not.toHaveBeenCalled() + await act(async () => root.unmount()) + }) +}) diff --git a/src/renderer/src/components/sidebar/RemoteFileBrowser.tsx b/src/renderer/src/components/sidebar/RemoteFileBrowser.tsx index d2e984b76de..07be1d9a3c1 100644 --- a/src/renderer/src/components/sidebar/RemoteFileBrowser.tsx +++ b/src/renderer/src/components/sidebar/RemoteFileBrowser.tsx @@ -1,12 +1,14 @@ import { ImeInput } from '@/lib/ime-text-field' import type React from 'react' -import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { useCallback, useEffect, useId, useMemo, useRef, useState } from 'react' import { LoaderCircle, Search } from 'lucide-react' import { Button } from '@/components/ui/button' import { cn } from '@/lib/utils' import { filterEntries, joinPath, parentPath, type DirEntry } from './remote-file-browser-helpers' import { RemoteFileBrowserBreadcrumbs } from './RemoteFileBrowserBreadcrumbs' import { RemoteFileBrowserEntryList } from './RemoteFileBrowserEntryList' +import { initialRemoteFileBrowserHighlight } from './remote-file-browser-highlight' +import type { PreviewState } from './remote-file-browser-path-preview-resolver' import { useRemoteFileBrowserFilterKeyCommands } from './use-remote-file-browser-filter-key-commands' import { useRemoteFileBrowserListing } from './use-remote-file-browser-listing' import { useRemoteFileBrowserPathPreview } from './use-remote-file-browser-path-preview' @@ -24,6 +26,7 @@ type RemoteFileBrowserProps = ( } const FILE_HINT_MS = 2000 +const NO_ENTRIES: readonly DirEntry[] = [] const FILE_HINT_TEXT = "Files can't be opened as a project" export function RemoteFileBrowser({ @@ -40,6 +43,15 @@ export function RemoteFileBrowser({ const [error, setError] = useState(null) const [filter, setFilter] = useState('') const [fileHint, setFileHint] = useState(false) + // Tied to the list, preview and text it was moved on, so any new listing, preview or keystroke starts + // from its own default (a cached listing reappears as the same array; a typed path waits on a debounce). + const [highlight, setHighlight] = useState<{ + entries: readonly DirEntry[] + preview: PreviewState | null + filter: string + index: number + } | null>(null) + const listboxId = useId() const genRef = useRef(0) const inputRef = useRef(null) const fileHintTimerRef = useRef | null>(null) @@ -61,7 +73,8 @@ export function RemoteFileBrowser({ handleInputPaste, discardPreview, resetPreviewForNavigation, - cancelPreviewWork + cancelPreviewWork, + previewMatchesInput } = useRemoteFileBrowserPathPreview({ resolvedPath, pathFlavor, @@ -156,6 +169,24 @@ export function RemoteFileBrowser({ }, [resolvedPath, navigate, pathFlavor]) const filteredEntries = useMemo(() => filterEntries(entries, filter), [entries, filter]) + const previewFilteredEntries = useMemo( + () => (preview ? filterEntries(preview.entries, preview.filter) : null), + [preview] + ) + // Why: while a listing loads only the spinner renders, so the prior rows can't be targeted. + const displayEntries = loading ? NO_ENTRIES : (previewFilteredEntries ?? filteredEntries) + // Why: a resolving preview keeps its prior rows on screen, but they are no longer targetable. + const activeIndex = preview?.loading + ? -1 + : highlight?.entries === displayEntries && + highlight.preview === preview && + highlight.filter === filter + ? highlight.index + : initialRemoteFileBrowserHighlight(displayEntries, preview !== null && preview.filter === '') + const moveHighlight = useCallback( + (index: number) => setHighlight({ entries: displayEntries, preview, filter, index }), + [displayEntries, preview, filter] + ) const triggerFileHint = useCallback(() => { if (fileHintTimerRef.current) { @@ -211,10 +242,16 @@ export function RemoteFileBrowser({ ) const handleFilterKeyDown = useRemoteFileBrowserFilterKeyCommands({ + loading, filter, setFilter, filteredEntries, + displayEntries, + activeIndex, + moveHighlight, + listParentPath, preview, + previewMatchesInput, discardPreview, resolvedPath, pathFlavor, @@ -247,6 +284,13 @@ export function RemoteFileBrowser({ handleInputChange(e.target.value)} @@ -256,6 +300,8 @@ export function RemoteFileBrowser({ 'auto.components.sidebar.RemoteFileBrowser.2300612806', 'Type to filter or enter a path…' )} + aria-controls={listboxId} + aria-activedescendant={activeIndex >= 0 ? `${listboxId}-${activeIndex}` : undefined} aria-invalid={!!preview?.error} aria-describedby={preview?.error ? 'remote-file-browser-path-error' : undefined} className={cn( @@ -283,7 +329,9 @@ export function RemoteFileBrowser({ loading={loading} error={error} entries={entries} - filteredEntries={filteredEntries} + displayEntries={displayEntries} + activeIndex={activeIndex} + listboxId={listboxId} filter={filter} preview={preview} isPreviewActive={isPreviewActive} diff --git a/src/renderer/src/components/sidebar/RemoteFileBrowserEntryList.tsx b/src/renderer/src/components/sidebar/RemoteFileBrowserEntryList.tsx index 6d8916d38bc..e5598091080 100644 --- a/src/renderer/src/components/sidebar/RemoteFileBrowserEntryList.tsx +++ b/src/renderer/src/components/sidebar/RemoteFileBrowserEntryList.tsx @@ -1,10 +1,9 @@ import type React from 'react' -import { useMemo, type RefObject } from 'react' +import { useEffect, useRef, type RefObject } from 'react' import { ChevronRight, Folder, LoaderCircle } from 'lucide-react' import { cn } from '@/lib/utils' import { FileTypeIcon } from '@/components/FileTypeIcon' import { - filterEntries, isRemoteFileBrowserPathResolveTextTooLarge, type DirEntry } from './remote-file-browser-helpers' @@ -15,7 +14,9 @@ type RemoteFileBrowserEntryListProps = { loading: boolean error: string | null entries: DirEntry[] - filteredEntries: DirEntry[] + displayEntries: readonly DirEntry[] + activeIndex: number + listboxId: string filter: string preview: PreviewState | null isPreviewActive: boolean @@ -28,7 +29,9 @@ export function RemoteFileBrowserEntryList({ loading, error, entries, - filteredEntries, + displayEntries, + activeIndex, + listboxId, filter, preview, isPreviewActive, @@ -36,12 +39,17 @@ export function RemoteFileBrowserEntryList({ handleRowClick, handleRowDoubleClick }: RemoteFileBrowserEntryListProps): React.JSX.Element { - const previewFilteredEntries = useMemo( - () => (preview ? filterEntries(preview.entries, preview.filter) : []), - [preview] - ) + const scrollRef = useRef(null) + useEffect(() => { + if (activeIndex < 0) { + return + } + // Optional call: happy-dom has no scrollIntoView. + scrollRef.current + ?.querySelector(`[data-entry-index="${activeIndex}"]`) + ?.scrollIntoView?.({ block: 'nearest' }) + }, [activeIndex, displayEntries]) - const displayEntries = isPreviewActive ? previewFilteredEntries : filteredEntries const displayEmptyDirCopy = isPreviewActive ? `${preview!.resolvedPath} is empty` : 'Empty directory' @@ -59,7 +67,16 @@ export function RemoteFileBrowserEntryList({ return (
-
+
{loading ? (
@@ -85,13 +102,19 @@ export function RemoteFileBrowserEntryList({ // Filtered-out entries need different copy from a directory that is genuinely empty.

{displayNoMatchesCopy}

-

{displayNoMatchesCopy}

) : ( - displayEntries.map((entry) => { + displayEntries.map((entry, index) => { + const isActive = index === activeIndex return (