mirror of
https://github.com/BradGroux/veritas-kanban.git
synced 2026-10-09 13:17:52 +00:00
Merge pull request #1562 from BradGroux/fix/board-keyboard-order-1533
fix: follow visible board order during keyboard navigation
This commit is contained in:
commit
fd15d29864
7 changed files with 129 additions and 33 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
<KeyboardProvider>
|
||||
<TestConsumer tasks={reordered} />
|
||||
</KeyboardProvider>
|
||||
);
|
||||
expect(screen.getByTestId('selected').textContent).toBe('later');
|
||||
fireEvent.keyDown(window, { key: 'ArrowDown' });
|
||||
expect(screen.getByTestId('selected').textContent).toBe('legacy');
|
||||
view.rerender(
|
||||
<KeyboardProvider>
|
||||
<TestConsumer tasks={[ranked, todo]} />
|
||||
</KeyboardProvider>
|
||||
);
|
||||
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(() => {});
|
||||
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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<HTMLDivElement | null>(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({
|
|||
}
|
||||
>
|
||||
<div
|
||||
ref={setNodeRef}
|
||||
ref={attachCard}
|
||||
data-task-id={task.id}
|
||||
style={style}
|
||||
{...(dragEnabled ? listeners : {})}
|
||||
|
|
@ -372,7 +387,7 @@ export const TaskCard = memo(function TaskCard({
|
|||
onMouseLeave={() => 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}
|
||||
|
|
|
|||
|
|
@ -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<KeyboardContextValue | null>(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<string | null>(null);
|
||||
const [tasks, setTasks] = useState<Task[]>([]);
|
||||
const [tasks, updateTasks] = useState<Task[]>([]);
|
||||
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<KeyboardContextValue>(
|
||||
() => ({
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue