-
Notifications
You must be signed in to change notification settings - Fork 27
[5/7] Open tasks in a modal over the board #276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
alex-clickhouse
wants to merge
3
commits into
alex-clickhouse/task-board-ui
from
alex-clickhouse/task-detail-modal
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| import { render, screen } from '@testing-library/react'; | ||
| import userEvent from '@testing-library/user-event'; | ||
| import { describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| import type { Task } from '../../api/client'; | ||
| import { TaskDetailBody } from './TaskDetailBody'; | ||
|
|
||
| vi.mock('../../api/client', () => ({ | ||
| api: { updateTask: vi.fn() }, | ||
| })); | ||
|
|
||
| /** | ||
| * The dirty guard is the one piece of behaviour here that a reader cannot | ||
| * verify by looking: it only shows itself when a re-render arrives while the | ||
| * textarea holds unsaved text. Now that the full page and the board's modal | ||
| * both render this component, a regression would silently discard typing in | ||
| * two places at once. | ||
| */ | ||
|
|
||
| function task(content: string): Task & { content: string } { | ||
| return { | ||
| id: 't1', title: 'Fix the encoder', status: 'pending', position: 1024, | ||
| deadline: null, source: 'manual', source_url: null, tags: '', | ||
| created_at: '2026-08-05T00:00:00Z', updated_at: '2026-08-05T00:00:00Z', | ||
| content, | ||
| }; | ||
| } | ||
|
|
||
| const editor = () => screen.getByPlaceholderText('Task content...'); | ||
|
|
||
| async function startEditing() { | ||
| await userEvent.click(screen.getByTitle('Edit')); | ||
| } | ||
|
|
||
| describe('TaskDetailBody content sync', () => { | ||
| it('adopts the task content when nothing has been typed', async () => { | ||
| const { rerender } = render(<TaskDetailBody task={task('# original')} />); | ||
| await startEditing(); | ||
| expect(editor()).toHaveValue('# original'); | ||
|
|
||
| rerender(<TaskDetailBody task={task('# rewritten elsewhere')} />); | ||
|
|
||
| // Clean editor: the newer content is strictly better than what is shown. | ||
| expect(editor()).toHaveValue('# rewritten elsewhere'); | ||
| }); | ||
|
|
||
| it('keeps unsaved edits when the task content changes underneath', async () => { | ||
| const { rerender } = render(<TaskDetailBody task={task('# original')} />); | ||
| await startEditing(); | ||
| await userEvent.type(editor(), ' plus my notes'); | ||
| expect(editor()).toHaveValue('# original plus my notes'); | ||
|
|
||
| rerender(<TaskDetailBody task={task('# rewritten elsewhere')} />); | ||
|
|
||
| // The whole point: an incoming update must not throw away typing. | ||
| expect(editor()).toHaveValue('# original plus my notes'); | ||
| }); | ||
|
|
||
| it('offers Save only once something has been typed', async () => { | ||
| render(<TaskDetailBody task={task('# original')} />); | ||
| await startEditing(); | ||
| expect(screen.queryByRole('button', { name: /save/i })).not.toBeInTheDocument(); | ||
|
|
||
| await userEvent.type(editor(), '!'); | ||
|
|
||
| expect(screen.getByRole('button', { name: /save/i })).toBeInTheDocument(); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| import { useCallback, useEffect, useState } from 'react'; | ||
| import { Calendar, Edit3, ExternalLink, Eye, Save } from 'lucide-react'; | ||
| import type { Task } from '../../api/client'; | ||
| import { useTaskStore } from '../../stores/taskStore'; | ||
| import { StatusBadge, StatusSelect } from './StatusControls'; | ||
| import { MarkdownContent } from '../Chat/MarkdownContent'; | ||
|
|
||
| /** | ||
| * The task editor itself — metadata row, edit/preview toggle, markdown | ||
| * body, save. | ||
| * | ||
| * Shared by the full page and the board's modal so the two can't drift. | ||
| * Only the surrounding chrome differs: the page supplies a back button and | ||
| * heading, the modal supplies the dialog frame. | ||
| */ | ||
|
alex-clickhouse marked this conversation as resolved.
|
||
| export function TaskDetailBody({ task }: { task: Task }) { | ||
| const saving = useTaskStore((s) => s.saving); | ||
| const saveTaskContent = useTaskStore((s) => s.saveTaskContent); | ||
| const updateStatus = useTaskStore((s) => s.updateStatus); | ||
|
|
||
| const [mode, setMode] = useState<'edit' | 'preview'>('preview'); | ||
| const [localContent, setLocalContent] = useState(''); | ||
| const [dirty, setDirty] = useState(false); | ||
|
|
||
| // Adopt the loaded markdown, but never clobber unsaved edits. | ||
| // | ||
| // Defensive rather than a fix for something reachable today: the only | ||
| // writers of `content` are the initial fetch and the user's own save, | ||
| // because a task_updated broadcast carries the `tasks` row and that table | ||
| // has no content column. The guard is here so that stops being a thing | ||
| // anyone has to know before adding one. | ||
| useEffect(() => { | ||
| if (task.content != null && !dirty) { | ||
| setLocalContent(task.content); | ||
| } | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [task.content]); | ||
|
|
||
| const handleSave = useCallback(async () => { | ||
| if (!dirty) return; | ||
| await saveTaskContent(task.id, localContent); | ||
| setDirty(false); | ||
| }, [task.id, dirty, localContent, saveTaskContent]); | ||
|
|
||
| const handleKeyDown = useCallback((e: React.KeyboardEvent) => { | ||
| if ((e.metaKey || e.ctrlKey) && e.key === 's') { | ||
| e.preventDefault(); | ||
| void handleSave(); | ||
| } | ||
| }, [handleSave]); | ||
|
|
||
| return ( | ||
| <div className="flex flex-col h-full min-h-0"> | ||
| <div className="flex items-center gap-3 px-5 py-2.5 text-[12px] border-b border-border-subtle shrink-0 flex-wrap"> | ||
| <StatusBadge status={task.status} /> | ||
| <StatusSelect | ||
| value={task.status} | ||
| onChange={(status) => updateStatus(task.id, status)} | ||
| className="text-[12px] px-2 py-1 bg-surface-raised border border-border rounded text-text-muted outline-none cursor-pointer" | ||
| /> | ||
| {task.deadline && ( | ||
| <span className="flex items-center gap-1 text-text-dim"> | ||
| <Calendar size={11} /> {task.deadline} | ||
| </span> | ||
| )} | ||
| {task.source && <span className="text-text-faint">from {task.source}</span>} | ||
| {task.source_url && ( | ||
| <a | ||
| href={task.source_url} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="flex items-center gap-1 text-accent hover:underline" | ||
| > | ||
| <ExternalLink size={11} /> source | ||
| </a> | ||
| )} | ||
|
|
||
| <div className="ml-auto flex items-center gap-2"> | ||
| {dirty && ( | ||
| <button | ||
| onClick={handleSave} | ||
| disabled={saving} | ||
| className="flex items-center gap-1.5 px-3 py-1 text-[12px] bg-accent hover:bg-accent-hover text-white rounded-md cursor-pointer disabled:opacity-50" | ||
| > | ||
| <Save size={12} /> {saving ? 'Saving...' : 'Save'} | ||
| </button> | ||
| )} | ||
| <div className="flex bg-surface-raised rounded-md border border-border"> | ||
| <button | ||
| onClick={() => setMode('edit')} | ||
| aria-pressed={mode === 'edit'} | ||
| title="Edit" | ||
| className={`px-2.5 py-1 rounded-l-md cursor-pointer transition-colors | ||
| ${mode === 'edit' ? 'text-text' : 'text-text-dim hover:text-text-muted'}`} | ||
| > | ||
| <Edit3 size={13} /> | ||
| </button> | ||
| <button | ||
| onClick={() => setMode('preview')} | ||
| aria-pressed={mode === 'preview'} | ||
| title="Preview" | ||
| className={`px-2.5 py-1 rounded-r-md cursor-pointer transition-colors | ||
| ${mode === 'preview' ? 'text-text' : 'text-text-dim hover:text-text-muted'}`} | ||
| > | ||
| <Eye size={13} /> | ||
| </button> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| {mode === 'edit' ? ( | ||
| <textarea | ||
| value={localContent} | ||
| onChange={(e) => { setLocalContent(e.target.value); setDirty(true); }} | ||
| onKeyDown={handleKeyDown} | ||
| className="flex-1 min-h-0 p-5 bg-bg-sunken text-[13px] text-text font-mono leading-relaxed outline-none resize-none" | ||
| spellCheck={false} | ||
| placeholder="Task content..." | ||
| /> | ||
| ) : ( | ||
| <div className="flex-1 min-h-0 overflow-y-auto px-6 py-5"> | ||
| {localContent | ||
| ? <MarkdownContent content={localContent} /> | ||
| : <span className="text-text-faint italic">No content</span>} | ||
| </div> | ||
| )} | ||
| </div> | ||
| ); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| import { useEffect } from 'react'; | ||
| import { useNavigate, useParams } from 'react-router-dom'; | ||
| import { Maximize2 } from 'lucide-react'; | ||
| import { useTaskStore } from '../../stores/taskStore'; | ||
| import { useTaskStatusStore } from '../../stores/taskStatusStore'; | ||
| import { Modal } from '../ui/Modal'; | ||
| import { TaskDetailBody } from './TaskDetailBody'; | ||
|
|
||
| /** | ||
| * `/tasks/:taskId` rendered over the board. | ||
| * | ||
| * Mounted only when the route was reached with a `background` location in | ||
| * history state (see `TasksPage.openTask`). A cold load or a refresh of | ||
| * the same URL has no such state and falls through to the full | ||
| * `TaskDetailPage` — so the URL stays shareable and the back button still | ||
| * means "back", while clicking a card doesn't tear down the board. | ||
| */ | ||
| export function TaskDetailModal() { | ||
| const { taskId } = useParams<{ taskId: string }>(); | ||
| const navigate = useNavigate(); | ||
|
|
||
| const selectedTask = useTaskStore((s) => s.selectedTask); | ||
| const detailLoading = useTaskStore((s) => s.detailLoading); | ||
| const loadTask = useTaskStore((s) => s.loadTask); | ||
| const clearSelectedTask = useTaskStore((s) => s.clearSelectedTask); | ||
| const loadStatuses = useTaskStatusStore((s) => s.load); | ||
|
|
||
| useEffect(() => { | ||
| if (taskId) void loadTask(taskId); | ||
| void loadStatuses(); | ||
| return () => clearSelectedTask(); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [taskId]); | ||
|
|
||
| // -1 rather than navigate('/tasks'): the board is still mounted behind | ||
| // this, so going back restores it with its scroll position intact. | ||
| const close = () => navigate(-1); | ||
|
|
||
| const title = detailLoading | ||
| ? 'Loading...' | ||
| : selectedTask?.title ?? 'Task not found'; | ||
|
|
||
| return ( | ||
| <Modal | ||
| open | ||
| onClose={close} | ||
| title={ | ||
| <span className="flex items-center gap-2 min-w-0"> | ||
| <span className="truncate">{title}</span> | ||
| {taskId && ( | ||
| <button | ||
| onClick={() => navigate(`/tasks/${taskId}`, { replace: true })} | ||
| title="Open as full page" | ||
| aria-label="Open as full page" | ||
| className="shrink-0 text-text-faint hover:text-text-muted cursor-pointer" | ||
| > | ||
| <Maximize2 size={13} /> | ||
| </button> | ||
| )} | ||
| </span> | ||
| } | ||
| // The task body is a document, not a form — it needs room to read. | ||
| size="wide" | ||
| // Markdown editing behind a backdrop click is too much to lose. | ||
| closeOnBackdrop={false} | ||
| className="h-[85vh] max-h-[85vh]" | ||
| > | ||
| {detailLoading && ( | ||
| <div className="p-8 text-center text-text-faint text-[13px]">Loading...</div> | ||
| )} | ||
| {!detailLoading && !selectedTask && ( | ||
| <div className="p-8 text-center text-text-faint text-[13px]">Task not found.</div> | ||
| )} | ||
| {!detailLoading && selectedTask && <TaskDetailBody task={selectedTask} />} | ||
| </Modal> | ||
| ); | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.