fix: arrow key navigation matches the visual grid
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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() {
|
||||
) : (
|
||||
<div className="w-4" />
|
||||
)}
|
||||
|
||||
|
||||
{/* Item Icon */}
|
||||
{item.icon && (
|
||||
<span className={clsx('flex-shrink-0', isSelected ? 'text-primary' : 'text-text-muted')}>
|
||||
{item.icon}
|
||||
</span>
|
||||
)}
|
||||
|
||||
|
||||
{/* Label (or inline rename input for folder rows) */}
|
||||
{renamingId === item.id ? (
|
||||
<input
|
||||
@@ -381,16 +381,16 @@ export function LeftSidebar() {
|
||||
) : (
|
||||
<span className="flex-1 truncate">{item.label}</span>
|
||||
)}
|
||||
|
||||
|
||||
{/* Count Badge */}
|
||||
{item.count !== undefined && item.count > 0 && (
|
||||
<span className="rounded bg-surface-offset px-1.5 py-0.5 text-xs text-text-muted">
|
||||
{item.count}
|
||||
</span>
|
||||
)}
|
||||
|
||||
|
||||
</div>
|
||||
|
||||
|
||||
{/* Render Children */}
|
||||
{hasChildren && isExpanded && (
|
||||
<div>
|
||||
@@ -400,23 +400,23 @@ export function LeftSidebar() {
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
return (
|
||||
<div className="flex h-full flex-col bg-surface">
|
||||
{/* Sidebar Header */}
|
||||
<div className="flex items-center justify-between border-b border-border px-3 py-2">
|
||||
<h2 className="text-sm font-semibold text-text">Library</h2>
|
||||
<h2 className="text-sm font-semibold text-text">Views</h2>
|
||||
<button className="rounded p-1 text-text-muted hover:bg-surface-2 hover:text-text">
|
||||
<MoreHorizontal className="h-4 w-4" />
|
||||
</button>
|
||||
</div>
|
||||
|
||||
|
||||
{/* Tree View */}
|
||||
<div className="flex-1 overflow-y-auto py-2">
|
||||
{libraryTree.map((item) => renderTreeItem(item))}
|
||||
<HeapsPanel />
|
||||
</div>
|
||||
|
||||
|
||||
{/* Bottom Actions */}
|
||||
{folderTree.length > 0 && (
|
||||
<div className="border-t border-border p-3">
|
||||
|
||||
@@ -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<TimelineItem, { type: 'row' }> => 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 (
|
||||
|
||||
Reference in New Issue
Block a user