From 5e10b12b139727bce0da218d876dd135ba5d632a Mon Sep 17 00:00:00 2001 From: dtoro Date: Wed, 8 Apr 2026 12:25:16 +0200 Subject: [PATCH] fix: arrow key navigation matches the visual grid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Timeline arrow keys moved by currentIndex ± columns in the FLAT photos array, but with date / tag grouping the rendered grid has half-full last rows for each group, so flat-index nav routinely landed in the wrong cell — and tag grouping (where one photo can appear in multiple groups) made it incoherent. Fix: navigate the actual visual grid the user sees. - New photoRows = items.filter(type='row') in visual order. The buildItems pipeline already chunks photos into row items of [1..columns] cells per group; this is exactly the rendered layout. - findActiveCell() walks photoRows looking for the activePhotoId and returns its (rowIndex, colIndex), or null if it isn't on screen. First-occurrence wins, which matches user intuition in the tag-grouped view. - New move(dr, dc) helper: Left/Right: walk col, wrap across row boundaries (so going Right off the end of a half-full row jumps to the next group's first row). Clamps at the very first/last cell. Up/Down: change row, then clamp the column to the destination row's actual width — moving down into a 2-cell row from col 3 lands on col 1, not nothing. - The four arrow handlers all funnel through move(); shift-arrow still calls selectRange with the destination cell's globalIndex so range selection works the same as a shift-click on that cell. - Headers are skipped automatically because they were never in photoRows. Edge cells, end-of-group, single-row groups, and tag-repeated photos all behave consistently. Pulled activePhotoId out of usePhotoStore (was already in the store but the Timeline component wasn't reading it). Effect deps updated to invalidate the listener whenever the visible grid changes. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/components/layout/LeftSidebar.tsx | 36 +++--- frontend/src/components/timeline/Timeline.tsx | 103 +++++++++++++----- 2 files changed, 96 insertions(+), 43 deletions(-) diff --git a/frontend/src/components/layout/LeftSidebar.tsx b/frontend/src/components/layout/LeftSidebar.tsx index 4cf93b5..ead1d9d 100644 --- a/frontend/src/components/layout/LeftSidebar.tsx +++ b/frontend/src/components/layout/LeftSidebar.tsx @@ -144,7 +144,7 @@ export function LeftSidebar() { } } } - + // Fetch the recursive folder tree (one root per active source root). const { data: folderTree = [] } = useFolderTreeQuery() @@ -178,11 +178,11 @@ export function LeftSidebar() { queryClient.invalidateQueries({ queryKey: ['photos'] }) }, }) - + const handleScanAll = () => { scanLibraryMutation.mutate() } - + const toggleExpanded = (id: string) => { const newExpanded = new Set(expandedItems) if (newExpanded.has(id)) { @@ -192,7 +192,7 @@ export function LeftSidebar() { } setExpandedItems(newExpanded) } - + // Recursively map a backend FolderTreeNode into our generic TreeItem. const folderNodeToTreeItem = (node: FolderTreeNode): TreeItem => ({ id: `folder-${node.id}`, @@ -228,7 +228,7 @@ export function LeftSidebar() { children: folderTree.map(folderNodeToTreeItem), }, ] - + // Derive whether a tree item is currently the "active" filter target. // Folder rows are selected when the filter store's folderId matches; the // library "All Photos" virtual node is selected when no folder/heap filter @@ -299,10 +299,10 @@ export function LeftSidebar() { onDoubleClick={ item.id.startsWith('folder-') ? (e) => { - e.stopPropagation() - setRenamingId(item.id) - setRenameDraft(item.label) - } + e.stopPropagation() + setRenamingId(item.id) + setRenameDraft(item.label) + } : undefined } onDragOver={acceptsDrop ? (e) => { @@ -345,14 +345,14 @@ export function LeftSidebar() { ) : (
)} - + {/* Item Icon */} {item.icon && ( {item.icon} )} - + {/* Label (or inline rename input for folder rows) */} {renamingId === item.id ? ( {item.label} )} - + {/* Count Badge */} {item.count !== undefined && item.count > 0 && ( {item.count} )} - +
- + {/* Render Children */} {hasChildren && isExpanded && (
@@ -400,23 +400,23 @@ export function LeftSidebar() {
) } - + return (
{/* Sidebar Header */}
-

Library

+

Views

- + {/* Tree View */}
{libraryTree.map((item) => renderTreeItem(item))}
- + {/* Bottom Actions */} {folderTree.length > 0 && (
diff --git a/frontend/src/components/timeline/Timeline.tsx b/frontend/src/components/timeline/Timeline.tsx index 1e400e1..c160db7 100644 --- a/frontend/src/components/timeline/Timeline.tsx +++ b/frontend/src/components/timeline/Timeline.tsx @@ -167,6 +167,7 @@ export function Timeline() { const { selectedPhotos, + activePhotoId, lastSelectedIndex, rangeStartIndex, selectPhoto, @@ -282,50 +283,102 @@ export function Timeline() { return () => window.removeEventListener('resize', measureWidth) }, []) - // Handle keyboard shortcuts for photo navigation. Operates on the flat - // photos array, so it ignores grouping. + // Photo rows in visual order — drops the header items so navigation + // walks the grid as the user sees it. Each row has cells of length + // [1..columns], the last row of a group can be short, and a single + // photo with multiple tags will appear in multiple rows. + const photoRows = useMemo( + () => items.filter((it): it is Extract => it.type === 'row'), + [items] + ) + + // Locate the active photo in the visual grid. Returns the FIRST + // (rowIndex, colIndex) where its id appears, since a tag-grouped view + // can repeat a photo across groups. Returns null when there's no + // active photo or it isn't currently rendered. + const findActiveCell = (): { row: number; col: number } | null => { + if (!activePhotoId) return null + for (let r = 0; r < photoRows.length; r++) { + const row = photoRows[r] + const c = row.cells.findIndex((cell) => cell.photo.id === activePhotoId) + if (c >= 0) return { row: r, col: c } + } + return null + } + + // Handle keyboard shortcuts for photo navigation. Operates on the + // grouped grid the user sees, so a half-full last row of a group + // doesn't make ArrowDown skip into the wrong place. useEffect(() => { const handleKeyDown = (e: KeyboardEvent) => { - if (photos.length === 0) return + if (photoRows.length === 0) return const target = e.target as HTMLElement | null if (target && (target.tagName === 'INPUT' || target.tagName === 'TEXTAREA')) { return } - const currentIndex = lastSelectedIndex ?? -1 + const move = (dr: number, dc: number) => { + const current = findActiveCell() ?? { row: 0, col: -1 } + let nextRow = current.row + let nextCol = current.col + dc + + if (dc !== 0) { + // Wrap left/right across row boundaries. + while (nextCol < 0 && nextRow > 0) { + nextRow -= 1 + nextCol = photoRows[nextRow].cells.length - 1 + } + while ( + nextRow < photoRows.length && + nextCol >= photoRows[nextRow].cells.length + ) { + if (nextRow === photoRows.length - 1) { + nextCol = photoRows[nextRow].cells.length - 1 + break + } + nextRow += 1 + nextCol = 0 + } + if (nextCol < 0) nextCol = 0 + } + + if (dr !== 0) { + nextRow += dr + if (nextRow < 0) nextRow = 0 + if (nextRow >= photoRows.length) nextRow = photoRows.length - 1 + // Clamp the column to the destination row's actual width so + // moving down into a half-full row lands on its last cell + // instead of nothing. + const rowLen = photoRows[nextRow].cells.length + if (nextCol >= rowLen) nextCol = rowLen - 1 + if (nextCol < 0) nextCol = 0 + } + + const dest = photoRows[nextRow]?.cells[nextCol] + if (!dest) return + if (e.shiftKey) { + selectRange(dest.globalIndex) + } else { + selectPhoto(dest.photo.id, dest.globalIndex) + } + } switch (e.key) { case 'ArrowUp': e.preventDefault() - if (currentIndex > columns - 1) { - const newIndex = currentIndex - columns - if (e.shiftKey) selectRange(newIndex) - else selectPhoto(photos[newIndex].id, newIndex) - } + move(-1, 0) break case 'ArrowDown': e.preventDefault() - if (currentIndex < photos.length - columns) { - const newIndex = Math.min(currentIndex + columns, photos.length - 1) - if (e.shiftKey) selectRange(newIndex) - else selectPhoto(photos[newIndex].id, newIndex) - } + move(1, 0) break case 'ArrowLeft': e.preventDefault() - if (currentIndex > 0) { - const newIndex = currentIndex - 1 - if (e.shiftKey) selectRange(newIndex) - else selectPhoto(photos[newIndex].id, newIndex) - } + move(0, -1) break case 'ArrowRight': e.preventDefault() - if (currentIndex < photos.length - 1) { - const newIndex = currentIndex + 1 - if (e.shiftKey) selectRange(newIndex) - else selectPhoto(photos[newIndex].id, newIndex) - } + move(0, 1) break case 'a': if (e.ctrlKey || e.metaKey) { @@ -347,7 +400,7 @@ export function Timeline() { window.addEventListener('keydown', handleKeyDown) return () => window.removeEventListener('keydown', handleKeyDown) // eslint-disable-next-line react-hooks/exhaustive-deps - }, [photos, selectedPhotos, lastSelectedIndex, columns]) + }, [photoRows, photos, selectedPhotos, activePhotoId]) if (isLoading) { return (