From 913abdd353cd188ed89c0662bb55e524f30c8857 Mon Sep 17 00:00:00 2001
From: Marko Djordjevic <marko.homoludens@gmail.com>
Date: Wed, 18 Feb 2026 20:32:20 +0100
Subject: [PATCH] code-review-fix task 11.3: add keyboard navigation and
 click-outside close to ChartSelector

---
 src/components/ChartSelector.tsx | 115 +++++++++++++++++++++++++++++--
 1 file changed, 108 insertions(+), 7 deletions(-)

diff --git a/src/components/ChartSelector.tsx b/src/components/ChartSelector.tsx
index e2eb9f3..2850dbd 100644
--- a/src/components/ChartSelector.tsx
+++ b/src/components/ChartSelector.tsx
@@ -1,6 +1,6 @@
 'use client';
 
-import { useState } from 'react';
+import { useState, useRef, useEffect, KeyboardEvent } from 'react';
 import { ChevronDown, Trash2 } from 'lucide-react';
 
 interface Chart {
@@ -24,9 +24,90 @@ export default function ChartSelector({
 }: ChartSelectorProps) {
   const [isOpen, setIsOpen] = useState(false);
   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);
 
+  // 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) {
     return (
       <p className="text-[10px] text-muted-foreground italic">
@@ -36,9 +117,17 @@ export default function ChartSelector({
   }
 
   return (
-    <div className="relative">
+    <div className="relative" ref={containerRef}>
       <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"
       >
         <span className="font-mono font-medium truncate">{activeChart?.name || 'Select chart'}</span>
@@ -46,13 +135,24 @@ export default function ChartSelector({
       </button>
 
       {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">
-          {charts.map((chart) => (
+        <div
+          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
               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' : ''
-              }`}
+              } ${focusedIndex === index ? 'ring-1 ring-inset ring-ring' : ''}`}
+              onMouseEnter={() => setFocusedIndex(index)}
             >
               <div
                 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"
                 title="Delete chart"
+                aria-label={`Delete chart ${chart.name}`}
               >
                 <Trash2 className="h-3 w-3" />
               </button>