code-review-fix task 11.3: add keyboard navigation and click-outside close to ChartSelector
This commit is contained in:
parent
3793e646cb
commit
913abdd353
1 changed files with 108 additions and 7 deletions
|
|
@ -1,6 +1,6 @@
|
||||||
'use client';
|
'use client';
|
||||||
|
|
||||||
import { useState } from 'react';
|
import { useState, useRef, useEffect, KeyboardEvent } from 'react';
|
||||||
import { ChevronDown, Trash2 } from 'lucide-react';
|
import { ChevronDown, Trash2 } from 'lucide-react';
|
||||||
|
|
||||||
interface Chart {
|
interface Chart {
|
||||||
|
|
@ -24,9 +24,90 @@ export default function ChartSelector({
|
||||||
}: ChartSelectorProps) {
|
}: ChartSelectorProps) {
|
||||||
const [isOpen, setIsOpen] = useState(false);
|
const [isOpen, setIsOpen] = useState(false);
|
||||||
const [confirmDeleteId, setConfirmDeleteId] = useState<number | null>(null);
|
const [confirmDeleteId, setConfirmDeleteId] = useState<number | null>(null);
|
||||||
|
const [focusedIndex, setFocusedIndex] = useState<number>(-1);
|
||||||
|
|
||||||
|
const containerRef = useRef<HTMLDivElement>(null);
|
||||||
|
const itemRefs = useRef<(HTMLDivElement | null)[]>([]);
|
||||||
|
|
||||||
const activeChart = charts.find((c) => c.id === activeChartId);
|
const activeChart = charts.find((c) => c.id === activeChartId);
|
||||||
|
|
||||||
|
// Click-outside handler
|
||||||
|
useEffect(() => {
|
||||||
|
function handleMouseDown(event: MouseEvent) {
|
||||||
|
if (
|
||||||
|
containerRef.current &&
|
||||||
|
!containerRef.current.contains(event.target as Node)
|
||||||
|
) {
|
||||||
|
setIsOpen(false);
|
||||||
|
setFocusedIndex(-1);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
document.addEventListener('mousedown', handleMouseDown);
|
||||||
|
return () => {
|
||||||
|
document.removeEventListener('mousedown', handleMouseDown);
|
||||||
|
};
|
||||||
|
}, []);
|
||||||
|
|
||||||
|
// Scroll focused item into view
|
||||||
|
useEffect(() => {
|
||||||
|
if (isOpen && focusedIndex >= 0 && itemRefs.current[focusedIndex]) {
|
||||||
|
itemRefs.current[focusedIndex]?.scrollIntoView({ block: 'nearest' });
|
||||||
|
}
|
||||||
|
}, [focusedIndex, isOpen]);
|
||||||
|
|
||||||
|
// Reset focused index when dropdown closes
|
||||||
|
useEffect(() => {
|
||||||
|
if (!isOpen) {
|
||||||
|
setFocusedIndex(-1);
|
||||||
|
}
|
||||||
|
}, [isOpen]);
|
||||||
|
|
||||||
|
function handleToggleKeyDown(event: KeyboardEvent<HTMLButtonElement>) {
|
||||||
|
if (event.key === 'Enter' || event.key === ' ') {
|
||||||
|
event.preventDefault();
|
||||||
|
setIsOpen((prev) => !prev);
|
||||||
|
if (!isOpen) {
|
||||||
|
setFocusedIndex(0);
|
||||||
|
}
|
||||||
|
} else if (event.key === 'Escape') {
|
||||||
|
setIsOpen(false);
|
||||||
|
setFocusedIndex(-1);
|
||||||
|
} else if (event.key === 'ArrowDown') {
|
||||||
|
event.preventDefault();
|
||||||
|
if (!isOpen) {
|
||||||
|
setIsOpen(true);
|
||||||
|
setFocusedIndex(0);
|
||||||
|
} else {
|
||||||
|
setFocusedIndex((prev) => Math.min(prev + 1, charts.length - 1));
|
||||||
|
}
|
||||||
|
} else if (event.key === 'ArrowUp') {
|
||||||
|
event.preventDefault();
|
||||||
|
if (isOpen) {
|
||||||
|
setFocusedIndex((prev) => Math.max(prev - 1, 0));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function handleListKeyDown(event: KeyboardEvent<HTMLDivElement>) {
|
||||||
|
if (event.key === 'ArrowDown') {
|
||||||
|
event.preventDefault();
|
||||||
|
setFocusedIndex((prev) => Math.min(prev + 1, charts.length - 1));
|
||||||
|
} else if (event.key === 'ArrowUp') {
|
||||||
|
event.preventDefault();
|
||||||
|
setFocusedIndex((prev) => Math.max(prev - 1, 0));
|
||||||
|
} else if (event.key === 'Enter' || event.key === ' ') {
|
||||||
|
event.preventDefault();
|
||||||
|
if (focusedIndex >= 0 && focusedIndex < charts.length) {
|
||||||
|
onSelectChart(charts[focusedIndex].id);
|
||||||
|
setIsOpen(false);
|
||||||
|
}
|
||||||
|
} else if (event.key === 'Escape') {
|
||||||
|
setIsOpen(false);
|
||||||
|
setFocusedIndex(-1);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if (charts.length === 0) {
|
if (charts.length === 0) {
|
||||||
return (
|
return (
|
||||||
<p className="text-[10px] text-muted-foreground italic">
|
<p className="text-[10px] text-muted-foreground italic">
|
||||||
|
|
@ -36,9 +117,17 @@ export default function ChartSelector({
|
||||||
}
|
}
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div className="relative">
|
<div className="relative" ref={containerRef}>
|
||||||
<button
|
<button
|
||||||
onClick={() => setIsOpen(!isOpen)}
|
onClick={() => {
|
||||||
|
setIsOpen(!isOpen);
|
||||||
|
if (!isOpen) {
|
||||||
|
setFocusedIndex(0);
|
||||||
|
}
|
||||||
|
}}
|
||||||
|
onKeyDown={handleToggleKeyDown}
|
||||||
|
aria-haspopup="listbox"
|
||||||
|
aria-expanded={isOpen}
|
||||||
className="w-full flex items-center justify-between px-2 py-1.5 text-xs rounded bg-secondary/50 hover:bg-secondary text-foreground transition-colors"
|
className="w-full flex items-center justify-between px-2 py-1.5 text-xs rounded bg-secondary/50 hover:bg-secondary text-foreground transition-colors"
|
||||||
>
|
>
|
||||||
<span className="font-mono font-medium truncate">{activeChart?.name || 'Select chart'}</span>
|
<span className="font-mono font-medium truncate">{activeChart?.name || 'Select chart'}</span>
|
||||||
|
|
@ -46,13 +135,24 @@ export default function ChartSelector({
|
||||||
</button>
|
</button>
|
||||||
|
|
||||||
{isOpen && (
|
{isOpen && (
|
||||||
<div className="absolute z-50 mt-1 w-full rounded-md border border-border bg-popover shadow-md max-h-48 overflow-y-auto">
|
<div
|
||||||
{charts.map((chart) => (
|
role="listbox"
|
||||||
|
aria-label="Chart list"
|
||||||
|
tabIndex={-1}
|
||||||
|
onKeyDown={handleListKeyDown}
|
||||||
|
className="absolute z-50 mt-1 w-full rounded-md border border-border bg-popover shadow-md max-h-48 overflow-y-auto"
|
||||||
|
>
|
||||||
|
{charts.map((chart, index) => (
|
||||||
<div
|
<div
|
||||||
key={chart.id}
|
key={chart.id}
|
||||||
className={`flex items-center justify-between px-2 py-1.5 text-xs hover:bg-accent cursor-pointer ${
|
ref={(el) => { itemRefs.current[index] = el; }}
|
||||||
|
role="option"
|
||||||
|
aria-selected={chart.id === activeChartId}
|
||||||
|
tabIndex={focusedIndex === index ? 0 : -1}
|
||||||
|
className={`flex items-center justify-between px-2 py-1.5 text-xs hover:bg-accent cursor-pointer outline-none ${
|
||||||
chart.id === activeChartId ? 'bg-accent/50' : ''
|
chart.id === activeChartId ? 'bg-accent/50' : ''
|
||||||
}`}
|
} ${focusedIndex === index ? 'ring-1 ring-inset ring-ring' : ''}`}
|
||||||
|
onMouseEnter={() => setFocusedIndex(index)}
|
||||||
>
|
>
|
||||||
<div
|
<div
|
||||||
className="flex-1 min-w-0"
|
className="flex-1 min-w-0"
|
||||||
|
|
@ -70,6 +170,7 @@ export default function ChartSelector({
|
||||||
}}
|
}}
|
||||||
className="ml-2 p-0.5 text-muted-foreground hover:text-destructive flex-shrink-0"
|
className="ml-2 p-0.5 text-muted-foreground hover:text-destructive flex-shrink-0"
|
||||||
title="Delete chart"
|
title="Delete chart"
|
||||||
|
aria-label={`Delete chart ${chart.name}`}
|
||||||
>
|
>
|
||||||
<Trash2 className="h-3 w-3" />
|
<Trash2 className="h-3 w-3" />
|
||||||
</button>
|
</button>
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue