diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fd2ccd..e91d9b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Archived-day editing (`ArchiveEditDialog`, `ArchivedTaskRow`) had several accessibility gaps and unmemoized re-render/re-parse patterns. The "Restore" button no longer grabs focus via `autoFocus` when the dialog opens, letting Radix's default dialog focus behavior take over instead of landing keyboard focus on a state-changing action before the user has read anything. The day-summary and per-task edit toggles now announce their expanded/collapsed state via `aria-expanded`/`aria-controls`. The tasks table's header cells gained `scope="col"` and the table itself an accessible name tied to the day being edited. Per-task delete now requires a same-row Confirm/Cancel step instead of removing the row on a single click, matching the destructive-action pattern used elsewhere (the task isn't persisted until "Save Changes," so this doesn't need the full undo-toast machinery). On the performance side, the day/task dirty-check (`tasksChanged`) and the `handleTaskSave`/`handleTaskDelete`/toggle-expand callbacks are now memoized (`useMemo`/`useCallback`), and `ArchivedTaskRow`/`MarkdownDisplay` are wrapped in `React.memo`, so expanding one task row no longer re-renders every sibling row or re-parses unrelated markdown + — `src/components/ArchiveEditDialog.tsx`, `src/components/ArchiveEditDialog.test.tsx`, `src/components/ArchivedTaskRow.tsx`, `src/components/ArchivedTaskRow.test.tsx`, `src/components/MarkdownDisplay.tsx` - `endDay` persisted the exact clock-out timestamp instead of rounding it to the nearest 15 minutes the way `startDay` already rounds the day's start time, so the archived day's `endTime` (and the last task's mirrored `endTime`) kept its raw seconds-precise value while `ArchiveEditDialog`'s Tasks table and day-summary form independently rounded the same values for display — producing a visible mismatch between the "posted"/persisted end time and what the edit dialog showed. `endDay` now rounds the effective end time at the source, so archived data and its display agree — `src/contexts/TimeTrackingContext.tsx`, `src/contexts/TimeTracking.test.tsx` - PWA manifest was duplicated three ways: `public/manifest.json` was an orphaned copy nothing linked to, and VitePWA's `manifest` option in `vite.config.ts` generated a second `manifest.webmanifest` that got injected into `dist/index.html` alongside the hand-maintained ``, leaving two manifest links in the built page. The actually-used `site.webmanifest` also pointed at screenshot files that don't exist (`desktop-1.png`/`mobile-1.png`). Deleted the orphaned JSON, set `manifest: false` on the VitePWA plugin, and fixed `site.webmanifest`'s screenshot list to reference the real files diff --git a/src/components/ArchiveEditDialog.tsx b/src/components/ArchiveEditDialog.tsx index 595a3ba..0160e60 100644 --- a/src/components/ArchiveEditDialog.tsx +++ b/src/components/ArchiveEditDialog.tsx @@ -1,4 +1,4 @@ -import React, { useState, useEffect } from "react"; +import React, { useState, useEffect, useMemo, useCallback } from "react"; import { useForm } from "react-hook-form"; import { zodResolver } from "@hookform/resolvers/zod"; import { z } from "zod"; @@ -189,8 +189,18 @@ export const ArchiveEditDialog: React.FC = ({ // of whether the day-summary editor was ever opened. Tasks are compared // against the rounded baseline (not the raw day.tasks) so the automatic // last-task rounding alone doesn't look like an unsaved change. - const tasksChanged = - JSON.stringify(tasks) !== JSON.stringify(getRoundedTasks(day.tasks)); + // getRoundedTasks is redefined every render; depending on day.tasks alone + // still recomputes exactly when the baseline actually changes. + const roundedBaselineTasks = useMemo( + () => getRoundedTasks(day.tasks), + // eslint-disable-next-line react-hooks/exhaustive-deps + [day.tasks], + ); + + const tasksChanged = useMemo( + () => JSON.stringify(tasks) !== JSON.stringify(roundedBaselineTasks), + [tasks, roundedBaselineTasks], + ); const hasChanges = dayFormIsDirty || tasksChanged; const parseTimeInput = (timeStr: string, baseDate: Date): Date => { @@ -305,18 +315,28 @@ export const ArchiveEditDialog: React.FC = ({ onClose(); }; - const handleTaskSave = (updatedTask: Task) => { - const updatedTasks = tasks.map((t) => - t.id === updatedTask.id ? updatedTask : t, - ); - setTasks(updatedTasks); - setExpandedTaskId(null); - }; + const handleTaskSave = useCallback( + (updatedTask: Task) => { + const updatedTasks = tasks.map((t) => + t.id === updatedTask.id ? updatedTask : t, + ); + setTasks(updatedTasks); + setExpandedTaskId(null); + }, + [tasks], + ); - const handleTaskDelete = (taskId: string) => { - const updatedTasks = tasks.filter((t) => t.id !== taskId); - setTasks(updatedTasks); - }; + const handleTaskDelete = useCallback( + (taskId: string) => { + const updatedTasks = tasks.filter((t) => t.id !== taskId); + setTasks(updatedTasks); + }, + [tasks], + ); + + const handleToggleExpand = useCallback((id: string) => { + setExpandedTaskId((cur) => (cur === id ? null : id)); + }, []); const handleCancel = () => { resetFormState(); @@ -344,7 +364,6 @@ export const ArchiveEditDialog: React.FC = ({ size="sm" aria-label="Restore this day" className="text-blue-11 hover:text-blue-12" - autoFocus > Restore @@ -430,6 +449,8 @@ export const ArchiveEditDialog: React.FC = ({ ? "Close day summary editor" : "Edit day summary" } + aria-expanded={isSummaryEditing} + aria-controls="day-summary-editor-region" > @@ -440,7 +461,7 @@ export const ArchiveEditDialog: React.FC = ({ - + {isSummaryEditing ? (
@@ -587,16 +608,16 @@ export const ArchiveEditDialog: React.FC = ({
- +
- Task - Category - Project/Client - Start Time - End Time - Duration - Actions + Task + Category + Project/Client + Start Time + End Time + Duration + Actions @@ -605,9 +626,7 @@ export const ArchiveEditDialog: React.FC = ({ key={task.id} task={task} isExpanded={expandedTaskId === task.id} - onToggleExpand={(id) => - setExpandedTaskId((cur) => (cur === id ? null : id)) - } + onToggleExpand={handleToggleExpand} onSave={handleTaskSave} onDelete={handleTaskDelete} categories={categories} diff --git a/src/components/ArchivedTaskRow.test.tsx b/src/components/ArchivedTaskRow.test.tsx index d40ec13..580bfda 100644 --- a/src/components/ArchivedTaskRow.test.tsx +++ b/src/components/ArchivedTaskRow.test.tsx @@ -76,6 +76,25 @@ describe("ArchivedTaskRow", () => { expect(screen.getByRole("button", { name: "Delete task" })).toBeInTheDocument(); }); + it("announces expanded state via aria-expanded/aria-controls on the Edit toggle", () => { + renderRow({ + task: baseTask, + isExpanded: true, + onToggleExpand: vi.fn(), + onSave: vi.fn(), + onDelete: vi.fn(), + categories, + projects, + }); + + const toggle = screen.getByRole("button", { name: "Close task editor" }); + expect(toggle).toHaveAttribute("aria-expanded", "true"); + expect(toggle).toHaveAttribute( + "aria-controls", + "archive-task-editor-task-1" + ); + }); + it("calls onToggleExpand with the task id when Edit is clicked", async () => { const onToggleExpand = vi.fn(); const user = userEvent.setup(); @@ -93,7 +112,7 @@ describe("ArchivedTaskRow", () => { expect(onToggleExpand).toHaveBeenCalledWith("task-1"); }); - it("calls onDelete with the task id when Delete is clicked", async () => { + it("requires a confirm step before calling onDelete", async () => { const onDelete = vi.fn(); const user = userEvent.setup(); renderRow({ @@ -107,9 +126,95 @@ describe("ArchivedTaskRow", () => { }); await user.click(screen.getByRole("button", { name: "Delete task" })); + expect(onDelete).not.toHaveBeenCalled(); + + await user.click( + screen.getByRole("button", { name: "Confirm delete task" }) + ); expect(onDelete).toHaveBeenCalledWith("task-1"); }); + it("reverts to the single delete button when Cancel delete task is clicked", async () => { + const onDelete = vi.fn(); + const user = userEvent.setup(); + renderRow({ + task: baseTask, + isExpanded: false, + onToggleExpand: vi.fn(), + onSave: vi.fn(), + onDelete, + categories, + projects, + }); + + await user.click(screen.getByRole("button", { name: "Delete task" })); + await user.click( + screen.getByRole("button", { name: "Cancel delete task" }) + ); + + expect(onDelete).not.toHaveBeenCalled(); + expect( + screen.getByRole("button", { name: "Delete task" }) + ).toBeInTheDocument(); + }); + + it("resets a mid-confirm delete state when the row collapses", async () => { + const user = userEvent.setup(); + const { rerender } = renderRow({ + task: baseTask, + isExpanded: true, + onToggleExpand: vi.fn(), + onSave: vi.fn(), + onDelete: vi.fn(), + categories, + projects, + }); + + await user.click(screen.getByRole("button", { name: "Delete task" })); + expect( + screen.getByRole("button", { name: "Confirm delete task" }) + ).toBeInTheDocument(); + + rerender( + +
+ + + +
+ + ); + rerender( + + + + + +
+
+ ); + + expect( + screen.getByRole("button", { name: "Delete task" }) + ).toBeInTheDocument(); + }); + it("pre-fills the expanded editor with times rounded to the nearest 15 minutes", () => { renderRow({ task: baseTask, diff --git a/src/components/ArchivedTaskRow.tsx b/src/components/ArchivedTaskRow.tsx index 548a71b..322a9c4 100644 --- a/src/components/ArchivedTaskRow.tsx +++ b/src/components/ArchivedTaskRow.tsx @@ -52,7 +52,7 @@ function formatTime12Hour(date: Date | undefined): string { return `${hours}:${minutes.toString().padStart(2, "0")} ${ampm}`; } -export const ArchivedTaskRow: React.FC = ({ +export const ArchivedTaskRow: React.FC = React.memo(({ task, isExpanded, onToggleExpand, @@ -68,6 +68,7 @@ export const ArchivedTaskRow: React.FC = ({ category: "none", }); const [timeData, setTimeData] = useState({ startTime: "", endTime: "" }); + const [confirmingDelete, setConfirmingDelete] = useState(false); useEffect(() => { if (isExpanded) { @@ -85,6 +86,9 @@ export const ArchivedTaskRow: React.FC = ({ endTime: task.endTime ? formatTimeForInput(task.endTime) : "", }); } + if (!isExpanded) { + setConfirmingDelete(false); + } }, [task, projects, isExpanded]); const category = categories.find((c) => c.id === task.category); @@ -175,6 +179,8 @@ export const ArchivedTaskRow: React.FC = ({ size="sm" variant="outline" aria-label={isExpanded ? "Close task editor" : "Edit task"} + aria-expanded={isExpanded} + aria-controls={`archive-task-editor-${task.id}`} > {isExpanded ? ( @@ -185,26 +191,48 @@ export const ArchivedTaskRow: React.FC = ({ {isExpanded ? "Close" : "Edit task"} - - + {confirmingDelete ? ( +
- - Delete task - + +
+ ) : ( + + + + + Delete task + + )}
{isExpanded && ( - +
@@ -370,4 +398,5 @@ export const ArchivedTaskRow: React.FC = ({ )} ); -}; +}); +ArchivedTaskRow.displayName = "ArchivedTaskRow"; diff --git a/src/components/MarkdownDisplay.tsx b/src/components/MarkdownDisplay.tsx index 5be30f7..e20ac1d 100644 --- a/src/components/MarkdownDisplay.tsx +++ b/src/components/MarkdownDisplay.tsx @@ -8,7 +8,7 @@ interface MarkdownDisplayProps { className?: string; } -export const MarkdownDisplay: React.FC = ({ content, className = "" }) => { +export const MarkdownDisplay: React.FC = React.memo(({ content, className = "" }) => { return (
= ({ content, class
); -}; +}); +MarkdownDisplay.displayName = "MarkdownDisplay";