Skip to content

Commit 6b4dfa0

Browse files
fix: address PR review — cleanup timers, AbortController, a11y, zoom keyboard support
- SpecDetailView: cleanup setTimeout on unmount via ref, add keyboard accessibility to zoom container (role, tabIndex, Enter/Space, focus-visible) - SpecTabs: add AbortController to tag counts fetch for unmount cleanup - PlotOfTheDay: add aria-label to dismiss button - Update Serena style_guide memory to reflect tokenized highlight colors Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 1c1387e commit 6b4dfa0

4 files changed

Lines changed: 20 additions & 5 deletions

File tree

.serena/memories/style_guide.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ Full scale: micro, xxs, xs, sm, md, base, lg, xl.
3737
- `tableStyle` — consistent table cells/headers
3838
- `labelStyle` — small labels (0.875rem, labelText)
3939

40-
## Highlight Colors (not tokenized)
41-
`#dbeafe`/`#1e40af` (highlighted tag chips) and `#90caf9` (tooltip text on dark bg) are intentionally kept as direct values.
40+
## Highlight Colors
41+
Use theme tokens for highlight treatments: `colors.highlight.bg`/`colors.highlight.text` for highlighted tag chips and `colors.tooltipLight` for tooltip text on dark backgrounds. Do not reintroduce hardcoded highlight hex values.
4242

4343
## Full reference
4444
See `docs/reference/style-guide.md` for complete documentation including spacing, breakpoints, component specs, animations, and accessibility rules.

app/src/components/PlotOfTheDay.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ export function PlotOfTheDay() {
9696
<IconButton
9797
onClick={handleDismiss}
9898
size="small"
99+
aria-label="Dismiss plot of the day"
99100
sx={{
100101
color: colors.gray[400], p: 0.25,
101102
'&:hover': { color: colors.gray[600] },

app/src/components/SpecDetailView.tsx

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* Shows large image with library carousel and action buttons.
55
*/
66

7-
import { useState, useCallback, useRef } from 'react';
7+
import { useState, useCallback, useRef, useEffect } from 'react';
88
import { Link } from 'react-router-dom';
99
import Box from '@mui/material/Box';
1010
import IconButton from '@mui/material/IconButton';
@@ -56,6 +56,7 @@ export function SpecDetailView({
5656
const [zoomed, setZoomed] = useState(false);
5757
const [origin, setOrigin] = useState({ x: 50, y: 50 });
5858
const [animating, setAnimating] = useState(false);
59+
const animTimerRef = useRef<ReturnType<typeof setTimeout>>(null);
5960
const prevLibRef = useRef(selectedLibrary);
6061

6162
// Reset zoom when library changes (no effect needed)
@@ -77,11 +78,16 @@ export function SpecDetailView({
7778
}
7879
setAnimating(true);
7980
setZoomed((z) => !z);
80-
setTimeout(() => setAnimating(false), 300);
81+
if (animTimerRef.current) clearTimeout(animTimerRef.current);
82+
animTimerRef.current = setTimeout(() => setAnimating(false), 300);
8183
},
8284
[zoomed],
8385
);
8486

87+
useEffect(() => {
88+
return () => { if (animTimerRef.current) clearTimeout(animTimerRef.current); };
89+
}, []);
90+
8591
const handleMouseMove = useCallback(
8692
(e: React.MouseEvent) => {
8793
if (!zoomed || animating || !containerRef.current) return;
@@ -116,7 +122,11 @@ export function SpecDetailView({
116122
>
117123
<Box
118124
ref={containerRef}
125+
role="button"
126+
tabIndex={0}
127+
aria-label={zoomed ? 'Zoom out' : 'Zoom in'}
119128
onClick={handleZoomToggle}
129+
onKeyDown={(e: React.KeyboardEvent) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); handleZoomToggle(e as unknown as React.MouseEvent); } }}
120130
onMouseMove={handleMouseMove}
121131
onTouchMove={handleTouchMove}
122132
sx={{
@@ -128,6 +138,8 @@ export function SpecDetailView({
128138
aspectRatio: '16/9',
129139
cursor: zoomed ? 'zoom-out' : 'zoom-in',
130140
touchAction: zoomed ? 'none' : 'auto',
141+
outline: 'none',
142+
'&:focus-visible': { boxShadow: `0 0 0 2px ${colors.primary}` },
131143
'&:hover .impl-counter': {
132144
opacity: 1,
133145
},

app/src/components/SpecTabs.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,8 @@ export function SpecTabs({
191191
// Fetch global tag counts once (module-level cache)
192192
useEffect(() => {
193193
if (cachedTagCounts) return;
194-
fetch(`${API_URL}/plots/filter?limit=1`)
194+
const controller = new AbortController();
195+
fetch(`${API_URL}/plots/filter?limit=1`, { signal: controller.signal })
195196
.then(r => r.ok ? r.json() : null)
196197
.then(data => {
197198
if (data?.globalCounts) {
@@ -200,6 +201,7 @@ export function SpecTabs({
200201
}
201202
})
202203
.catch(() => {});
204+
return () => controller.abort();
203205
}, []);
204206

205207
// Get count for a tag value (e.g., "scatter" in "plot" category → 421 implementations)

0 commit comments

Comments
 (0)