Skip to content

fix: Code quality and accessibility improvements #63

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.

Sign up for GitHub

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

Merged
AdamJ merged 2 commits into from
Mar 23, 2026
Merged

fix: Code quality and accessibility improvements #63

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0153a7c
fix: code quality and accessibility improvements
claude Mar 22, 2026
220e636
docs: update CHANGELOG with code quality and accessibility entries
claude Mar 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Accessibility
- Added `aria-label` to all icon-only buttons whose visible text label is hidden on mobile viewports: Restore and Edit in `ArchiveItem`, Restore/Delete/Edit in `ArchiveEditDialog` header, per-task Edit/Delete in `ArchiveEditDialog` task table, and Edit/Delete in `ProjectManagement`
- Replaced `focus:outline-none` with `focus-visible:outline-none` + `focus-visible:ring-2 focus-visible:ring-ring` on Radix `TabsTrigger` elements in `ArchiveItem` — the browser focus ring was previously stripped for all input methods; it is now suppressed only for pointer clicks while remaining fully visible for keyboard navigation
- Connected `Label`/`Textarea` pairs via `htmlFor`/`id` in `ArchiveEditDialog` (day notes field) and `TaskEditInArchiveDialog` (task description field) so screen readers announce the field label when the input receives focus

### Code Quality
- Replaced hardcoded Tailwind gray color classes (`text-gray-900`, `text-gray-600/500/400`) with theme variables (`text-foreground`, `text-muted-foreground`) across `ArchiveItem`, `ArchiveEditDialog`, `TaskEditInArchiveDialog`, `ProjectManagement`, and `ExportDialog`
- Replaced hardcoded red color classes (`text-red-*`, `bg-red-*`, `border-red-*`) with `text-destructive`, `bg-destructive/5`, and `border-destructive/20` in the `ArchiveEditDialog` delete confirmation card, `ExportDialog` import error alert, and `TaskEditInArchiveDialog` required-field indicator; switched icon-only Delete buttons to `variant="destructive"` in `ArchiveEditDialog` and `ProjectManagement`
- Replaced `data-[state=active]:border-blue-600` with `data-[state=active]:border-primary` on tab triggers in `ArchiveItem`; replaced `bg-gray-50 dark:bg-gray-800` note/summary panels with `bg-muted`
- Fixed inline style violations: `style={{ marginBottom: '1rem' }}` → `className="mb-4"` in `ArchiveEditDialog`; `style={{ display: 'none' }}` → `className="hidden"` in `ExportDialog`

### Performance
- **Archive page summary stats** (`src/pages/Archive.tsx`): Wrapped the four summary-stat `reduce()` calls (total hours, billable hours, non-billable hours, revenue) in a single `useMemo` keyed on `[filteredDays, projects, categories]`; previously they recomputed on every render regardless of whether the underlying data had changed. Also replaced unstable context method wrappers with stable module-level imports from `calculationUtils` so the memo invalidates only when data actually changes.
- **Archive item row rendering** (`src/components/ArchiveItem.tsx`): Per-day stats (`hoursWorked`, `billableHours`, `nonBillableHours`, `revenue`) are now computed in a `useMemo([day, projects, categories])` instead of inline on every render; also eliminates the duplicate `getRevenueForDay()` call that previously appeared twice in the revenue badge. Project and category lookups in the task table now use `Map.get()` (O(1)) instead of `Array.find()` (O(n)) per row via `useMemo`-cached lookup maps. The daily-summary `generateDailySummary()` call is memoized on `day.tasks` so it only runs when task descriptions change.
Expand Down
45 changes: 25 additions & 20 deletions src/components/ArchiveEditDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ export const ArchiveEditDialog: React.FC<ArchiveEditDialogProps> = ({
onClick={handleRestoreDay}
variant="outline"
size="sm"
aria-label="Restore this day"
className="text-blue-600 hover:text-blue-700"
>
<RotateCcw className="w-4 h-4" />
Expand All @@ -274,6 +275,7 @@ export const ArchiveEditDialog: React.FC<ArchiveEditDialogProps> = ({
onClick={() => setShowDeleteConfirm(true)}
variant="destructive"
size="sm"
aria-label="Delete this day"
>
<Trash2 className="w-4 h-4" />
<span className="hidden md:block md:ml-2">Delete</span>
Expand All @@ -282,6 +284,7 @@ export const ArchiveEditDialog: React.FC<ArchiveEditDialogProps> = ({
onClick={() => setIsEditing(true)}
variant="default"
size="sm"
aria-label="Edit this day"
>
<Edit className="w-4 h-4" />
<span className="hidden md:block md:ml-2">Edit</span>
Expand Down Expand Up @@ -312,7 +315,7 @@ export const ArchiveEditDialog: React.FC<ArchiveEditDialogProps> = ({
</Callout.Text>
</Callout.Root>
)}
<Card style={{ marginBottom: '1rem' }}>
<Card className="mb-4">
<CardHeader>
<CardTitle className="text-lg">Summary of Day</CardTitle>
</CardHeader>
Expand Down Expand Up @@ -355,14 +358,15 @@ export const ArchiveEditDialog: React.FC<ArchiveEditDialogProps> = ({
</div>
</div>
<div>
<Label>Notes</Label>
<Label htmlFor="day-notes">Notes</Label>
<Tabs defaultValue="edit" className="w-full">
<TabsList className="grid w-full grid-cols-2">
<TabsTrigger value="edit">Edit</TabsTrigger>
<TabsTrigger value="preview">Preview</TabsTrigger>
</TabsList>
<TabsContent value="edit">
<Textarea
id="day-notes"
value={dayData.notes}
onChange={e =>
setDayData(prev => ({
Expand Down Expand Up @@ -391,21 +395,21 @@ export const ArchiveEditDialog: React.FC<ArchiveEditDialogProps> = ({
) : (
<div className="grid grid-cols-1 sm:grid-cols-2 gap-y-4 gap-x-8 text-sm">
<div>
<span className="font-medium text-gray-900">
<span className="font-medium text-foreground">
Start Time:
</span>
<span className="ml-2 text-gray-600">
<span className="ml-2 text-muted-foreground">
{formatTime12Hour(day.startTime)}
</span>
</div>
<div>
<span className="font-medium text-gray-900">End Time:</span>
<span className="ml-2 text-gray-600">
<span className="font-medium text-foreground">End Time:</span>
<span className="ml-2 text-muted-foreground">
{formatTime12Hour(day.endTime)}
</span>
</div>
<div>
<span className="font-medium text-gray-900">
<span className="font-medium text-foreground">
Total Duration:
</span>
<div className="flex items-center space-x-2 mt-1">
Expand All @@ -416,15 +420,15 @@ export const ArchiveEditDialog: React.FC<ArchiveEditDialogProps> = ({
</div>
</div>
<div>
<span className="font-medium text-gray-900">Tasks:</span>
<span className="ml-2 text-gray-600">
<span className="font-medium text-foreground">Tasks:</span>
<span className="ml-2 text-muted-foreground">
{tasks.length} total
</span>
</div>
{day.notes && (
<div className="col-span-2">
<span className="font-medium text-gray-900">Notes:</span>
<div className="mt-1 text-gray-600">
<span className="font-medium text-foreground">Notes:</span>
<div className="mt-1 text-muted-foreground">
<MarkdownDisplay content={day.notes} />
</div>
</div>
Expand Down Expand Up @@ -464,7 +468,7 @@ export const ArchiveEditDialog: React.FC<ArchiveEditDialogProps> = ({
<div>{task.title}</div>
<span className="hidden lg:block">
{task.description && (
<div className="text-sm text-gray-500 mt-1">
<div className="text-sm text-muted-foreground mt-1">
<MarkdownDisplay
content={task.description}
/>
Expand All @@ -491,7 +495,7 @@ export const ArchiveEditDialog: React.FC<ArchiveEditDialogProps> = ({
{task.project}
</div>
{task.client && (
<div className="text-xs text-gray-500">
<div className="text-xs text-muted-foreground">
{task.client}
</div>
)}
Expand All @@ -516,14 +520,15 @@ export const ArchiveEditDialog: React.FC<ArchiveEditDialogProps> = ({
onClick={() => handleTaskEdit(task)}
size="sm"
variant="outline"
aria-label="Edit task"
>
<Edit className="w-3 h-3" />
</Button>
<Button
onClick={() => handleTaskDelete(task.id)}
size="sm"
variant="outline"
className="text-red-600 hover:text-red-700"
variant="destructive"
aria-label="Delete task"
>
<Trash2 className="w-3 h-3" />
</Button>
Expand All @@ -541,15 +546,15 @@ export const ArchiveEditDialog: React.FC<ArchiveEditDialogProps> = ({

{/* Delete Confirmation */}
{showDeleteConfirm && (
<Card className="border-red-200 bg-red-50">
<Card className="border-destructive/20 bg-destructive/5">
<CardContent className="p-4">
<div className="flex items-center space-x-3">
<AlertTriangle className="w-5 h-5 text-red-600" />
<AlertTriangle className="w-5 h-5 text-destructive" />
<div className="flex-1">
<h4 className="font-medium text-red-900">
<h4 className="font-medium text-destructive">
Delete Archived Day
</h4>
<p className="text-sm text-red-700 mt-1">
<p className="text-sm text-destructive mt-1">
Are you sure you want to delete this archived day? This
action cannot be undone.
</p>
Expand All @@ -558,7 +563,7 @@ export const ArchiveEditDialog: React.FC<ArchiveEditDialogProps> = ({
<Button
onClick={handleDeleteDay}
size="sm"
className="bg-red-600 hover:bg-red-700 text-white"
variant="destructive"
>
Delete
</Button>
Expand Down
34 changes: 18 additions & 16 deletions src/components/ArchiveItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ export const ArchiveItem: React.FC<ArchiveItemProps> = ({ day, onEdit }) => {
onClick={handleRestore}
variant="ghost"
size="sm"
aria-label="Restore this day"
className="flex items-center space-x-2 text-blue-600 hover:text-blue-700"
>
<RotateCcw className="w-4 h-4 block" />
Expand All @@ -96,6 +97,7 @@ export const ArchiveItem: React.FC<ArchiveItemProps> = ({ day, onEdit }) => {
onClick={() => onEdit(day)}
variant="default"
size="sm"
aria-label="Edit this day"
className="flex items-center space-x-2"
>
<Edit className="w-4 h-4 block" />
Expand All @@ -111,10 +113,10 @@ export const ArchiveItem: React.FC<ArchiveItemProps> = ({ day, onEdit }) => {
<div className="space-y-2">
<div className="flex items-center justify-between text-sm print:text-base">
<div className="flex items-center space-x-4">
<span className="text-gray-600 print:text-black">
<span className="text-muted-foreground print:text-black">
Started: {formatTime(day.startTime)}
</span>
<span className="text-gray-600 print:text-black">
<span className="text-muted-foreground print:text-black">
Ended: {formatTime(day.endTime)}
</span>
</div>
Expand All @@ -138,7 +140,7 @@ export const ArchiveItem: React.FC<ArchiveItemProps> = ({ day, onEdit }) => {
<span className="text-green-600 print:text-black font-medium">
Billable: {dayStats.billableHours.toFixed(2)}h
</span>
<span className="text-gray-600 print:text-black font-medium">
<span className="text-muted-foreground print:text-black font-medium">
Non-billable: {dayStats.nonBillableHours.toFixed(2)}h
</span>
</div>
Expand All @@ -154,42 +156,42 @@ export const ArchiveItem: React.FC<ArchiveItemProps> = ({ day, onEdit }) => {
{/* Daily Summary */}
{dailySummary && (
<div className="space-y-2 border-t pt-4">
<h4 className="font-medium text-gray-900 flex items-center mb-2">
<h4 className="font-medium text-foreground flex items-center mb-2">
<FileText className="w-4 h-4 mr-2" />
Overview
</h4>
<Tabs defaultValue="summary" className="w-full">
<TabsList className="border-b mb-4">
<TabsTrigger
value="summary"
className="px-3 py-1 text-sm font-medium text-gray-700 data-[state=active]:border-blue-600 data-[state=active]:border-b-2 focus:outline-none"
className="px-3 py-1 text-sm font-medium text-muted-foreground data-[state=active]:border-primary data-[state=active]:border-b-2 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring"
>
Summary
</TabsTrigger>
<TabsTrigger
value="notes"
className="px-3 py-1 text-sm font-medium text-gray-700 data-[state=active]:border-blue-600 data-[state=active]:border-b-2 focus:outline-none"
className="px-3 py-1 text-sm font-medium text-muted-foreground data-[state=active]:border-primary data-[state=active]:border-b-2 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring"
>
Notes
</TabsTrigger>
</TabsList>
<TabsContent value="summary" className="focus:outline-none">
<div className="bg-gray-50 dark:bg-gray-800 p-4 rounded-md print:bg-white print:border print:border-gray-300">
<TabsContent value="summary" className="focus-visible:outline-none">
<div className="bg-muted p-4 rounded-md print:bg-white print:border print:border-border">
<MarkdownDisplay
content={dailySummary}
className="prose-p:text-gray-700 dark:prose-p:text-gray-300 print:prose-p:text-gray-800"
className="prose-p:text-muted-foreground print:prose-p:text-foreground"
/>
</div>
</TabsContent>
<TabsContent value="notes" className="focus:outline-none">
<div className="bg-gray-50 dark:bg-gray-800 p-4 rounded-md print:bg-white print:border print:border-gray-300">
<TabsContent value="notes" className="focus-visible:outline-none">
<div className="bg-muted p-4 rounded-md print:bg-white print:border print:border-border">
{day.notes ? (
<MarkdownDisplay
content={day.notes}
className="prose-p:text-gray-700 dark:prose-p:text-gray-300 print:prose-p:text-gray-800"
className="prose-p:text-muted-foreground print:prose-p:text-foreground"
/>
) : (
<div className="prose-sm prose-p:leading-relaxed prose-p:my-1 prose-p:text-gray-700 dark:prose-p:text-gray-300 print:prose-p:text-gray-800">
<div className="prose-sm prose-p:leading-relaxed prose-p:my-1 prose-p:text-muted-foreground print:prose-p:text-foreground">
<p>No notes for this day have been entered.</p>
</div>
)}
Expand All @@ -201,7 +203,7 @@ export const ArchiveItem: React.FC<ArchiveItemProps> = ({ day, onEdit }) => {

{/* Tasks Table */}
<div className="print:mt-2">
<h4 className="font-medium text-gray-900 print:hidden mb-2">
<h4 className="font-medium text-foreground print:hidden mb-2">
Tasks ({day.tasks.length})
</h4>
<Table>
Expand Down Expand Up @@ -249,7 +251,7 @@ export const ArchiveItem: React.FC<ArchiveItemProps> = ({ day, onEdit }) => {
<TableRow key={task.id} className="print:border-black">
<TableCell className="font-medium print:text-black">
{task.title}
<div className="text-sm text-gray-400 hidden md:block print:text-gray-600">
<div className="text-sm text-muted-foreground hidden md:block print:text-foreground">
<MarkdownDisplay
content={task.description}
className="prose-sm line-clamp-1 hover:line-clamp-none transition-all duration-200"
Expand All @@ -259,7 +261,7 @@ export const ArchiveItem: React.FC<ArchiveItemProps> = ({ day, onEdit }) => {
<TableCell className="print:text-black">
{task.project || '-'}
{project?.hourlyRate && (
<div className="text-xs text-gray-500 print:text-gray-600">
<div className="text-xs text-muted-foreground print:text-foreground">
${project.hourlyRate}/hr
</div>
)}
Expand Down
Loading
Loading