fix: address component review feedback — types, a11y, SSR, and key stability (#10)

* Initial plan

* fix: address PR review feedback on components

- DataTable: import type ReactNode, fix React.ReactNode refs, add keyboard-accessible sort buttons
- Sidebar: import type KeyboardEvent, fix React.KeyboardEvent ref
- Modal: useId() for unique title IDs, SSR guard before createPortal, stable action keys
- ErrorBoundary.test: explicitly import beforeEach/afterEach from vitest

Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>

Author: Copilot

package-lock.json

@@ -1,4 +1,4 @@
-import { useState, type CSSProperties } from 'react';
+import { useState, type CSSProperties, type ReactNode } from 'react';
 import { colors, baseStyles } from '../theme';
 
 export interface DataTableColumn<T> {
@@ -9,7 +9,7 @@ export interface DataTableColumn<T> {
   /** Whether this column is sortable. Default: false */
   sortable?: boolean;
   /** Optional custom cell renderer. */
-  render?: (value: T[keyof T], row: T) => React.ReactNode;
+  render?: (value: T[keyof T], row: T) => ReactNode;
   /** Optional column header style override. */
   headerStyle?: CSSProperties;
   /** Optional cell style override. */
@@ -114,6 +114,7 @@ export default function DataTable<T extends Record<string, unknown>>({
     fontFamily: "'Inter', 'Segoe UI', system-ui, sans-serif",
     ...style,
   };
+  const thPadding = baseStyles.th.padding;
 
   return (
     <div style={containerStyle}>
@@ -126,10 +127,9 @@ export default function DataTable<T extends Record<string, unknown>>({
                   key={col.key}
                   style={{
                     ...baseStyles.th,
-                    cursor: col.sortable ? 'pointer' : 'default',
+                    padding: 0,
                     ...col.headerStyle,
                   }}
-                  onClick={() => handleSort(col)}
                   aria-sort={
                     sortState?.key === col.key
                       ? sortState.direction === 'asc'
@@ -138,12 +138,32 @@ export default function DataTable<T extends Record<string, unknown>>({
                       : undefined
                   }
                 >
-                  {col.label}
-                  {col.sortable && (
-                    <SortIcon
-                      direction={sortState?.key === col.key ? sortState.direction : undefined}
-                      active={sortState?.key === col.key}
-                    />
+                  {col.sortable ? (
+                    <button
+                      onClick={() => handleSort(col)}
+                      style={{
+                        background: 'none',
+                        border: 'none',
+                        cursor: 'pointer',
+                        padding: thPadding,
+                        width: '100%',
+                        textAlign: 'left',
+                        font: 'inherit',
+                        color: 'inherit',
+                        display: 'flex',
+                        alignItems: 'center',
+                      }}
+                    >
+                      {col.label}
+                      <SortIcon
+                        direction={sortState?.key === col.key ? sortState.direction : undefined}
+                        active={sortState?.key === col.key}
+                      />
+                    </button>
+                  ) : (
+                    <span style={{ display: 'block', padding: thPadding }}>
+                      {col.label}
+                    </span>
                   )}
                 </th>
               ))}
@@ -173,7 +193,7 @@ export default function DataTable<T extends Record<string, unknown>>({
                     <td key={col.key} style={{ ...baseStyles.td, ...col.cellStyle }}>
                       {col.render
                         ? col.render(row[col.key], row)
-                        : (row[col.key] as React.ReactNode)}
+                        : (row[col.key] as ReactNode)}
                     </td>
                   ))}
                 </tr>

src/components/DataTable.tsx

@@ -1,4 +1,4 @@
-import { describe, it, expect, vi } from 'vitest';
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 import { render, screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import ErrorBoundary from './ErrorBoundary';

src/components/ErrorBoundary.test.tsx

@@ -1,5 +1,6 @@
 import {
   useEffect,
+  useId,
   useRef,
   type CSSProperties,
   type ReactNode,
@@ -9,6 +10,8 @@ import { createPortal } from 'react-dom';
 import { colors, baseStyles } from '../theme';
 
 export interface ModalAction {
+  /** Stable unique identifier for this action button (used as React key). */
+  id?: string;
   /** Button label. */
   label: string;
   /** Click handler. */
@@ -46,6 +49,7 @@ export default function Modal({
   style,
 }: ModalProps) {
   const dialogRef = useRef<HTMLDivElement>(null);
+  const titleId = useId();
 
   // Focus trap: focus the dialog on open.
   useEffect(() => {
@@ -136,7 +140,7 @@ export default function Modal({
         ref={dialogRef}
         role="dialog"
         aria-modal="true"
-        aria-labelledby={title ? 'modal-title' : undefined}
+        aria-labelledby={title ? titleId : undefined}
         tabIndex={-1}
         style={dialogStyle}
         onClick={(e) => e.stopPropagation()}
@@ -145,7 +149,7 @@ export default function Modal({
         {(title != null) && (
           <div style={headerStyle}>
             <h2
-              id="modal-title"
+              id={titleId}
               style={{
                 margin: 0,
                 fontSize: '18px',
@@ -179,9 +183,9 @@ export default function Modal({
 
         {actions && actions.length > 0 && (
           <div style={footerStyle}>
-            {actions.map((action) => (
+            {actions.map((action, idx) => (
               <button
-                key={action.label}
+                key={action.id ?? idx}
                 onClick={action.onClick}
                 disabled={action.disabled}
                 style={{
@@ -199,5 +203,9 @@ export default function Modal({
     </div>
   );
 
+  if (typeof document === 'undefined') {
+    return null;
+  }
+
   return createPortal(modal, document.body);
 }

src/components/Modal.tsx

@@ -1,4 +1,4 @@
-import type { CSSProperties, ReactNode } from 'react';
+import type { CSSProperties, ReactNode, KeyboardEvent } from 'react';
 import { colors } from '../theme';
 
 export interface SidebarItem {
@@ -184,7 +184,7 @@ function SidebarNavItem({ item, collapsed, onNavigate }: SidebarNavItemProps) {
     if (!isDisabled) onNavigate(item);
   }
 
-  function handleKeyDown(e: React.KeyboardEvent) {
+  function handleKeyDown(e: KeyboardEvent<HTMLDivElement>) {
     if (e.key === 'Enter' || e.key === ' ') {
       e.preventDefault();
       handleClick();