From 3d2e693b2b6b5c19944e46e04f91fb06e7574dce Mon Sep 17 00:00:00 2001 From: bradgroux Date: Mon, 7 Sep 2026 13:47:48 -0500 Subject: [PATCH] fix: navigate tasks in visible board order --- docs/FEATURES.md | 2 +- web/src/__tests__/KanbanBoard.test.tsx | 24 ++++++++---- web/src/__tests__/TaskCard.test.tsx | 13 ++++++ web/src/__tests__/useKeyboard.test.tsx | 50 +++++++++++++++++++++++- web/src/components/board/KanbanBoard.tsx | 15 +++++-- web/src/components/task/TaskCard.tsx | 21 ++++++++-- web/src/hooks/useKeyboard.tsx | 37 ++++++++++-------- 7 files changed, 129 insertions(+), 33 deletions(-) diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 89a67b45..16149865 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -122,7 +122,7 @@ The Kanban board is the central interface — a drag-and-drop workspace that ref - **Mobile shell controls** — Compact navigation uses bounded labels and full accessible names; Board Chat stays fixed above the bottom navigation and device safe area - **Resizable Workbench** — Board Chat and Squad Chat open in one bounded right-side dock, preserve the active conversation when switching channels, and clamp their width to keep the application shell recoverable - **Bulk operations** — Select multiple tasks to move, archive, or delete in batch; select-all toggle -- **Keyboard shortcuts** — Navigate tasks (j/k, arrows), open (Enter), close (Esc), create (c), move to column (1-4), help (?) +- **Keyboard shortcuts** — Navigate visible tasks in saved board order (j/k, arrows), focus and reveal the selected card, open (Enter), close (Esc), create (c), move to configured column (1-9), help (?) - **Loading skeleton** — Shimmer placeholders while the board loads - **Blocked column** — Dedicated column for blocked tasks with categorized reasons (waiting on feedback, technical snag, prerequisite, other) - **Comments** — Add, edit, and delete comments on tasks with author attribution and relative timestamps diff --git a/web/src/__tests__/KanbanBoard.test.tsx b/web/src/__tests__/KanbanBoard.test.tsx index d118db93..a69901bf 100644 --- a/web/src/__tests__/KanbanBoard.test.tsx +++ b/web/src/__tests__/KanbanBoard.test.tsx @@ -89,14 +89,13 @@ vi.mock('@/hooks/useAgentStatus', () => ({ }), })); -vi.mock('@/hooks/useKeyboard', () => ({ - useKeyboard: () => ({ - selectedTaskId: null, - setTasks: vi.fn(), - setOnOpenTask: vi.fn(), - setOnMoveTask: vi.fn(), - }), +const keyboardRegistration = vi.hoisted(() => ({ + selectedTaskId: null, + setTasks: vi.fn(), + setOnOpenTask: vi.fn(), + setOnMoveTask: vi.fn(), })); +vi.mock('@/hooks/useKeyboard', () => ({ useKeyboard: () => keyboardRegistration })); vi.mock('@/hooks/useFeatureSettings', () => ({ useFeatureSettings: () => mockFeatureSettingsResult, @@ -310,6 +309,17 @@ afterEach(() => { // ── Tests ──────────────────────────────────────────────────── describe('KanbanBoard', () => { + it('removes task navigation and callbacks when the board unmounts', () => { + mockUseTasks = () => ({ data: mockTasks, isLoading: false, error: null }); + const view = renderBoard(); + expect(keyboardRegistration.setOnOpenTask).toHaveBeenLastCalledWith(expect.any(Function)); + expect(keyboardRegistration.setOnMoveTask).toHaveBeenLastCalledWith(expect.any(Function)); + view.unmount(); + expect(keyboardRegistration.setTasks).toHaveBeenLastCalledWith([]); + expect(keyboardRegistration.setOnOpenTask).toHaveBeenLastCalledWith(null); + expect(keyboardRegistration.setOnMoveTask).toHaveBeenLastCalledWith(null); + }); + it('shows loading skeleton when data is loading', () => { mockUseTasks = () => ({ data: undefined, isLoading: true, error: null }); renderBoard(); diff --git a/web/src/__tests__/TaskCard.test.tsx b/web/src/__tests__/TaskCard.test.tsx index 2960bc7d..de114397 100644 --- a/web/src/__tests__/TaskCard.test.tsx +++ b/web/src/__tests__/TaskCard.test.tsx @@ -156,6 +156,19 @@ describe('TaskCard', () => { cleanup(); }); + it('focuses and reveals keyboard selection with an accessible label', () => { + ensureMantineBrowserApis(); + const scroll = vi.spyOn(Element.prototype, 'scrollIntoView').mockImplementation(() => {}); + try { + renderCard(createMockTask({ title: 'Keyboard target' }), { isSelected: true }); + const card = screen.getByRole('article', { name: /^Selected\. Task: Keyboard target/ }); + expect(document.activeElement).toBe(card); + expect(scroll).toHaveBeenCalledWith({ block: 'nearest', inline: 'nearest' }); + } finally { + scroll.mockRestore(); + } + }); + it('renders task title', () => { const task = createMockTask({ title: 'Implement login' }); renderCard(task); diff --git a/web/src/__tests__/useKeyboard.test.tsx b/web/src/__tests__/useKeyboard.test.tsx index c542477a..05874753 100644 --- a/web/src/__tests__/useKeyboard.test.tsx +++ b/web/src/__tests__/useKeyboard.test.tsx @@ -4,7 +4,7 @@ import React from 'react'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { render, screen, fireEvent, cleanup } from '@testing-library/react'; -import type { Task, TaskStatus } from '@veritas-kanban/shared'; +import { taskBoardRankAtIndex, type Task, type TaskStatus } from '@veritas-kanban/shared'; import { createMockTask } from './test-utils'; // Mock toast — vi.mock is hoisted before imports. @@ -100,6 +100,54 @@ describe('KeyboardProvider', () => { cleanup(); }); + it('follows positions and durable ranks across custom columns, reorder, and filtering', () => { + featureSettingsMock.settings.board.columns = [ + { id: 'ready', title: 'Ready' }, + { id: 'todo', title: 'To Do' }, + ]; + const legacy = createMockTask({ id: 'legacy', title: 'Zulu', status: 'ready', position: 1 }); + const later = createMockTask({ id: 'later', title: 'Alpha', status: 'ready', position: 5 }); + const ranked = createMockTask({ + id: 'ranked', + title: 'Middle', + status: 'ready', + position: 99, + boardRank: taskBoardRankAtIndex([legacy, later], 1), + }); + const todo = createMockTask({ id: 'todo', status: 'todo', position: -100 }); + const hidden = createMockTask({ id: 'hidden', status: 'retired', position: -200 }); + const tasks = [todo, later, ranked, legacy, hidden]; + const view = renderWithProvider({ tasks }); + for (const id of ['legacy', 'ranked', 'later', 'todo']) { + fireEvent.keyDown(window, { key: 'j' }); + expect(screen.getByTestId('selected').textContent).toBe(id); + } + fireEvent.keyDown(window, { key: 'ArrowUp' }); + expect(screen.getByTestId('selected').textContent).toBe('later'); + const reordered = [ + todo, + { ...later, boardRank: taskBoardRankAtIndex([legacy, ranked], 0) }, + ranked, + legacy, + ]; + view.rerender( + + + + ); + expect(screen.getByTestId('selected').textContent).toBe('later'); + fireEvent.keyDown(window, { key: 'ArrowDown' }); + expect(screen.getByTestId('selected').textContent).toBe('legacy'); + view.rerender( + + + + ); + expect(screen.getByTestId('selected').textContent).toBe('none'); + fireEvent.keyDown(window, { key: 'k' }); + expect(screen.getByTestId('selected').textContent).toBe('todo'); + }); + it('throws when useKeyboard is used outside provider', () => { vi.spyOn(console, 'error').mockImplementation(() => {}); diff --git a/web/src/components/board/KanbanBoard.tsx b/web/src/components/board/KanbanBoard.tsx index bfd847b8..927a75a2 100644 --- a/web/src/components/board/KanbanBoard.tsx +++ b/web/src/components/board/KanbanBoard.tsx @@ -375,7 +375,6 @@ export function KanbanBoard() { // Register filtered tasks with keyboard context useEffect(() => { setTasks(filteredTasks); - return () => setTasks([]); }, [filteredTasks, setTasks]); // Handler for opening a task @@ -532,9 +531,17 @@ export function KanbanBoard() { [allTasksByStatus, announce, canWriteTasks, columns, commitBoardMove, filteredTasks, isOnline] ); - // Register callbacks with keyboard context (refs, so no need for useEffect) - setOnOpenTask(handleTaskClick); - setOnMoveTask(handleMoveTask); + // Board-owned callbacks must not outlive this view. + useEffect(() => { + setOnOpenTask(handleTaskClick); + setOnMoveTask(handleMoveTask); + return () => { + setOnOpenTask(null); + setOnMoveTask(null); + }; + }, [handleTaskClick, handleMoveTask, setOnOpenTask, setOnMoveTask]); + + useEffect(() => () => setTasks([]), [setTasks]); // Drag and drop logic const { diff --git a/web/src/components/task/TaskCard.tsx b/web/src/components/task/TaskCard.tsx index 668516fd..96ff520f 100644 --- a/web/src/components/task/TaskCard.tsx +++ b/web/src/components/task/TaskCard.tsx @@ -1,4 +1,4 @@ -import { memo, useMemo, useState } from 'react'; +import { memo, useMemo, useState, useRef, useEffect, useCallback } from 'react'; import { Select, Tooltip } from '@mantine/core'; import { useSortable } from '@dnd-kit/sortable'; import { CSS } from '@dnd-kit/utilities'; @@ -218,6 +218,21 @@ export const TaskCard = memo(function TaskCard({ id: task.id, disabled: !dragEnabled, }); + const cardRef = useRef(null); + const attachCard = useCallback( + (node: HTMLDivElement | null) => { + cardRef.current = node; + setNodeRef(node); + }, + [setNodeRef] + ); + useEffect(() => { + if (isSelected) { + cardRef.current?.focus({ preventScroll: true }); + cardRef.current?.scrollIntoView?.({ block: 'nearest', inline: 'nearest' }); + } + }, [isSelected]); + const { isSelecting, toggleSelect, isSelected: isBulkSelected } = useBulkActions(); const [tooltipDismissed, setTooltipDismissed] = useState(false); const [statusMenuOpen, setStatusMenuOpen] = useState(false); @@ -362,7 +377,7 @@ export const TaskCard = memo(function TaskCard({ } >
setTooltipDismissed(false)} role="article" tabIndex={0} - aria-label={`Task: ${task.title}, Type: ${typeLabel}, Priority: ${task.priority}${readinessAria}${isBlockedState ? ', Blocked' : ''}${isAgentRunning ? ', Agent running' : ''}${isAttemptFailed ? ', Latest attempt failed' : ''}${isAwaitingReview ? ', Awaiting review' : ''}${isVerified ? ', Verified' : ''}`} + aria-label={`${isSelected ? 'Selected. ' : ''}Task: ${task.title}, Type: ${typeLabel}, Priority: ${task.priority}${readinessAria}${isBlockedState ? ', Blocked' : ''}${isAgentRunning ? ', Agent running' : ''}${isAttemptFailed ? ', Latest attempt failed' : ''}${isAwaitingReview ? ', Awaiting review' : ''}${isVerified ? ', Verified' : ''}`} data-type-color-token={typeColorToken} data-selected={isSelected ? 'true' : undefined} data-dragging={isDragging || isCurrentlyDragging ? 'true' : undefined} diff --git a/web/src/hooks/useKeyboard.tsx b/web/src/hooks/useKeyboard.tsx index abd842ee..c29f4619 100644 --- a/web/src/hooks/useKeyboard.tsx +++ b/web/src/hooks/useKeyboard.tsx @@ -11,6 +11,7 @@ import { import { DEFAULT_FEATURE_SETTINGS, normalizeBoardColumns, + sortTasksByBoardPosition, type Task, type TaskStatus, } from '@veritas-kanban/shared'; @@ -38,8 +39,8 @@ interface KeyboardContextValue { setTasks: (tasks: Task[]) => void; // Callbacks (using refs to avoid re-render loops) - setOnOpenTask: (fn: (task: Task) => void) => void; - setOnMoveTask: (fn: (taskId: string, status: TaskStatus) => void) => void; + setOnOpenTask: (fn: ((task: Task) => void) | null) => void; + setOnMoveTask: (fn: ((taskId: string, status: TaskStatus) => void) | null) => void; } const KeyboardContext = createContext(null); @@ -52,7 +53,11 @@ function getColumnForShortcut(key: string, columns: Array<{ id: TaskStatus }>): export function KeyboardProvider({ children }: { children: ReactNode }) { const [isHelpOpen, setIsHelpOpen] = useState(false); const [selectedTaskId, setSelectedTaskId] = useState(null); - const [tasks, setTasks] = useState([]); + const [tasks, updateTasks] = useState([]); + const setTasks = useCallback((next: Task[]) => { + updateTasks(next); + setSelectedTaskId((id) => (id && next.some((task) => task.id === id) ? id : null)); + }, []); const { settings } = useFeatureSettings(); const columns = useMemo( () => normalizeBoardColumns(settings.board?.columns ?? DEFAULT_FEATURE_SETTINGS.board.columns), @@ -89,24 +94,23 @@ export function KeyboardProvider({ children }: { children: ReactNode }) { setIsHelpOpen(false); }, []); - const setOnOpenTask = useCallback((fn: (task: Task) => void) => { + const setOnOpenTask = useCallback((fn: ((task: Task) => void) | null) => { onOpenTaskRef.current = fn; }, []); - const setOnMoveTask = useCallback((fn: (taskId: string, status: TaskStatus) => void) => { + const setOnMoveTask = useCallback((fn: ((taskId: string, status: TaskStatus) => void) | null) => { onMoveTaskRef.current = fn; }, []); - // Get flat list of tasks sorted by column then position - const getTaskList = useCallback(() => { - const statusOrder = columns.map((column) => column.id); - return [...tasks].sort((a, b) => { - const aIndex = statusOrder.indexOf(a.status); - const bIndex = statusOrder.indexOf(b.status); - if (aIndex !== bIndex) return aIndex - bIndex; - return a.title.localeCompare(b.title); - }); - }, [columns, tasks]); + // Match the rendered column order and the board's canonical rank/position order. + // Compute once per snapshot, rather than sorting during each keystroke. + const taskList = useMemo( + () => + columns.flatMap((column) => + sortTasksByBoardPosition(tasks.filter((task) => task.status === column.id)) + ), + [columns, tasks] + ); // Keyboard event handler useEffect(() => { @@ -141,7 +145,6 @@ export function KeyboardProvider({ children }: { children: ReactNode }) { return; } - const taskList = getTaskList(); const currentIndex = selectedTaskId ? taskList.findIndex((t) => t.id === selectedTaskId) : -1; // Cmd+Shift+C (or Ctrl+Shift+C on Windows/Linux) - Toggle chat panel @@ -237,7 +240,7 @@ export function KeyboardProvider({ children }: { children: ReactNode }) { window.addEventListener('keydown', handleKeyDown); return () => window.removeEventListener('keydown', handleKeyDown); - }, [getTaskList, selectedTaskId, isHelpOpen, openCreateDialog, openChatPanel, columns]); + }, [taskList, selectedTaskId, isHelpOpen, openCreateDialog, openChatPanel, columns]); const value = useMemo( () => ({