refactor(time-picker): draw the hour, minute and AM/PM columns with one spinner column

The three columns of the time picker each repeated the same chevron up, value box and chevron down markup. TimeSpinnerColumn draws one, and the button style it shares with the clear button moves to timeSpinnerStyle. The AM/PM column keeps its narrower box, smaller type and leading gap as props, and the hour and minute boxes keep their tabular digits.
2026-10-18 21:16:47 +00:00 · 2026-10-09 15:12:43 +02:00
parent c1a9e98ec4
commit d08c949b14
4 changed files with 153 additions and 65 deletions
@@ -1,9 +1,14 @@
 import React, { useState, useRef, useEffect } from 'react'
 import { createPortal } from 'react-dom'
-import { Clock, ChevronUp, ChevronDown } from 'lucide-react'
+import { Clock } from 'lucide-react'
 import { useSettingsStore } from '../../store/settingsStore'
 import { formatClockTime, parseMeridiemTime } from '../../utils/formatters'
 import { useAnchoredPosition } from '../../hooks/useAnchoredPosition'
+import { TimeSpinnerColumn } from './TimeSpinnerColumn'
+import { spinnerButtonStyle } from './timeSpinnerStyle'
+
+// The hour and minute boxes, wide enough for two digits that do not jump as they change.
+const DIGITS: React.CSSProperties = { width: 44, fontSize: 'calc(22px * var(--fs-scale-title, 1))', fontVariantNumeric: 'tabular-nums' }

 interface CustomTimePickerProps {
  value: string
@@ -72,10 +77,9 @@ export default function CustomTimePicker({ value, onChange, placeholder = '00:00
    update(newH, newM)
  }

-  const btnStyle: React.CSSProperties = {
-    background: 'none', border: 'none', cursor: 'pointer', padding: 2,
-    color: 'var(--text-faint)', display: 'flex', borderRadius: 4,
-    transition: 'color 0.15s',
+  const toggleMeridiem = () => {
+    if (hour === null) return
+    if (hour < 12) update(hour + 12, minute ?? 0); else update(hour - 12, minute ?? 0)
  }

  const handleInput = (e: React.ChangeEvent<HTMLInputElement>) => {
@@ -179,78 +183,29 @@ export default function CustomTimePicker({ value, onChange, placeholder = '00:00
          backdropFilter: 'blur(24px)', WebkitBackdropFilter: 'blur(24px)',
        }}>
          {/* Hours */}
-          <div style={{ display: 'flex', flexDirection: 'column', alignItems: 'center', gap: 2 }}>
-            <button type="button" onClick={incHour} style={btnStyle}
-              onMouseEnter={e => e.currentTarget.style.color = 'var(--text-primary)'}
-              onMouseLeave={e => e.currentTarget.style.color = 'var(--text-faint)'}>
-              <ChevronUp size={16} />
-            </button>
-            <div style={{
-              width: 44, height: 40, display: 'flex', alignItems: 'center', justifyContent: 'center',
-              fontSize: 'calc(22px * var(--fs-scale-title, 1))', fontWeight: 700, color: 'var(--text-primary)',
-              background: 'var(--bg-hover)', borderRadius: 8,
-              fontVariantNumeric: 'tabular-nums',
-            }}>
-              {hour !== null ? (is12h ? String(hour === 0 ? 12 : hour > 12 ? hour - 12 : hour) : String(hour).padStart(2, '0')) : '--'}
-            </div>
-            <button type="button" onClick={decHour} style={btnStyle}
-              onMouseEnter={e => e.currentTarget.style.color = 'var(--text-primary)'}
-              onMouseLeave={e => e.currentTarget.style.color = 'var(--text-faint)'}>
-              <ChevronDown size={16} />
-            </button>
-          </div>
+          <TimeSpinnerColumn onUp={incHour} onDown={decHour} valueStyle={DIGITS}>
+            {hour !== null ? (is12h ? String(hour === 0 ? 12 : hour > 12 ? hour - 12 : hour) : String(hour).padStart(2, '0')) : '--'}
+          </TimeSpinnerColumn>

          <span style={{ fontSize: 'calc(22px * var(--fs-scale-title, 1))', fontWeight: 700, color: 'var(--text-faint)', marginTop: -2 }}>:</span>

          {/* Minutes */}
-          <div style={{ display: 'flex', flexDirection: 'column', alignItems: 'center', gap: 2 }}>
-            <button type="button" onClick={incMin} style={btnStyle}
-              onMouseEnter={e => e.currentTarget.style.color = 'var(--text-primary)'}
-              onMouseLeave={e => e.currentTarget.style.color = 'var(--text-faint)'}>
-              <ChevronUp size={16} />
-            </button>
-            <div style={{
-              width: 44, height: 40, display: 'flex', alignItems: 'center', justifyContent: 'center',
-              fontSize: 'calc(22px * var(--fs-scale-title, 1))', fontWeight: 700, color: 'var(--text-primary)',
-              background: 'var(--bg-hover)', borderRadius: 8,
-              fontVariantNumeric: 'tabular-nums',
-            }}>
-              {minute !== null ? String(minute).padStart(2, '0') : '--'}
-            </div>
-            <button type="button" onClick={decMin} style={btnStyle}
-              onMouseEnter={e => e.currentTarget.style.color = 'var(--text-primary)'}
-              onMouseLeave={e => e.currentTarget.style.color = 'var(--text-faint)'}>
-              <ChevronDown size={16} />
-            </button>
-          </div>
+          <TimeSpinnerColumn onUp={incMin} onDown={decMin} valueStyle={DIGITS}>
+            {minute !== null ? String(minute).padStart(2, '0') : '--'}
+          </TimeSpinnerColumn>

          {/* AM/PM Toggle */}
          {is12h && hour !== null && (
-            <div style={{ display: 'flex', flexDirection: 'column', alignItems: 'center', gap: 2, marginInlineStart: 4 }}>
-              <button type="button" onClick={() => { if (hour < 12) update(hour + 12, minute ?? 0); else update(hour - 12, minute ?? 0) }} style={btnStyle}
-                onMouseEnter={e => e.currentTarget.style.color = 'var(--text-primary)'}
-                onMouseLeave={e => e.currentTarget.style.color = 'var(--text-faint)'}>
-                <ChevronUp size={16} />
-              </button>
-              <div style={{
-                width: 36, height: 40, display: 'flex', alignItems: 'center', justifyContent: 'center',
-                fontSize: 'calc(14px * var(--fs-scale-body, 1))', fontWeight: 700, color: 'var(--text-primary)',
-                background: 'var(--bg-hover)', borderRadius: 8,
-              }}>
-                {hour >= 12 ? 'PM' : 'AM'}
-              </div>
-              <button type="button" onClick={() => { if (hour < 12) update(hour + 12, minute ?? 0); else update(hour - 12, minute ?? 0) }} style={btnStyle}
-                onMouseEnter={e => e.currentTarget.style.color = 'var(--text-primary)'}
-                onMouseLeave={e => e.currentTarget.style.color = 'var(--text-faint)'}>
-                <ChevronDown size={16} />
-              </button>
-            </div>
+            <TimeSpinnerColumn onUp={toggleMeridiem} onDown={toggleMeridiem} style={{ marginInlineStart: 4 }}
+              valueStyle={{ width: 36, fontSize: 'calc(14px * var(--fs-scale-body, 1))' }}>
+              {hour >= 12 ? 'PM' : 'AM'}
+            </TimeSpinnerColumn>
          )}

          {/* Clear */}
          {value && (
            <button type="button" onClick={() => { onChange(''); setOpen(false) }}
-              style={{ ...btnStyle, marginInlineStart: 4, fontSize: 'calc(11px * var(--fs-scale-caption, 1))', color: 'var(--text-faint)', padding: '4px 6px' }}
+              style={{ ...spinnerButtonStyle, marginInlineStart: 4, fontSize: 'calc(11px * var(--fs-scale-caption, 1))', color: 'var(--text-faint)', padding: '4px 6px' }}
              onMouseEnter={e => e.currentTarget.style.color = '#ef4444'}
              onMouseLeave={e => e.currentTarget.style.color = 'var(--text-faint)'}>
              ✕
@@ -0,0 +1,64 @@
+import { fireEvent, render, screen } from '../../../tests/helpers/render';
+import { TimeSpinnerColumn } from './TimeSpinnerColumn';
+import { spinnerButtonStyle } from './timeSpinnerStyle';
+
+describe('TimeSpinnerColumn', () => {
+  function renderColumn(props: Partial<Parameters<typeof TimeSpinnerColumn>[0]> = {}) {
+    const onUp = vi.fn();
+    const onDown = vi.fn();
+    render(
+      <TimeSpinnerColumn onUp={onUp} onDown={onDown} valueStyle={{ width: 44 }} {...props}>
+        07
+      </TimeSpinnerColumn>
+    );
+    const [up, down] = screen.getAllByRole('button');
+    return { onUp, onDown, up, down };
+  }
+
+  it('FE-COMP-TIMESPINNER-001: shows the value between an up and a down button', () => {
+    const { up, down } = renderColumn();
+    expect(screen.getByText('07')).toBeInTheDocument();
+    expect(up.compareDocumentPosition(screen.getByText('07')) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy();
+    expect(down.compareDocumentPosition(screen.getByText('07')) & Node.DOCUMENT_POSITION_PRECEDING).toBeTruthy();
+  });
+
+  it('FE-COMP-TIMESPINNER-002: the up button steps up and the down button steps down', () => {
+    const { onUp, onDown, up, down } = renderColumn();
+    fireEvent.click(up);
+    expect(onUp).toHaveBeenCalledTimes(1);
+    expect(onDown).not.toHaveBeenCalled();
+    fireEvent.click(down);
+    expect(onDown).toHaveBeenCalledTimes(1);
+    expect(onUp).toHaveBeenCalledTimes(1);
+  });
+
+  it('FE-COMP-TIMESPINNER-003: the buttons are plain buttons that do not submit a form', () => {
+    const { up, down } = renderColumn();
+    expect(up).toHaveAttribute('type', 'button');
+    expect(down).toHaveAttribute('type', 'button');
+  });
+
+  it('FE-COMP-TIMESPINNER-004: a hovered button lights up and dims again when the pointer leaves', () => {
+    const { up } = renderColumn();
+    expect(up.style.color).toBe(spinnerButtonStyle.color);
+    fireEvent.mouseEnter(up);
+    expect(up.style.color).toBe('var(--text-primary)');
+    fireEvent.mouseLeave(up);
+    expect(up.style.color).toBe('var(--text-faint)');
+  });
+
+  it('FE-COMP-TIMESPINNER-005: the value box takes its size from valueStyle over the shared box style', () => {
+    renderColumn({ valueStyle: { width: 36, height: 50 } });
+    const box = screen.getByText('07');
+    expect(box.style.width).toBe('36px');
+    expect(box.style.height).toBe('50px');
+    expect(box.style.fontWeight).toBe('700');
+  });
+
+  it('FE-COMP-TIMESPINNER-006: the column style adds to the column layout', () => {
+    renderColumn({ style: { marginInlineStart: 4 } });
+    const column = screen.getByText('07').parentElement as HTMLElement;
+    expect(column.style.marginInlineStart).toBe('4px');
+    expect(column.style.flexDirection).toBe('column');
+  });
+});
@@ -0,0 +1,56 @@
+import { ChevronDown, ChevronUp } from 'lucide-react';
+import type { CSSProperties, ReactNode } from 'react';
+import { spinnerButtonStyle } from './timeSpinnerStyle';
+
+interface TimeSpinnerColumnProps {
+  onUp: () => void;
+  onDown: () => void;
+  children: ReactNode;
+  /** Extra style for the column, such as the gap before the AM/PM column. */
+  style?: CSSProperties;
+  /** Size and type of the value box between the chevrons. */
+  valueStyle: CSSProperties;
+}
+
+/** One column of the time picker: a chevron up, the value, a chevron down. */
+export function TimeSpinnerColumn({ onUp, onDown, children, style, valueStyle }: TimeSpinnerColumnProps) {
+  return (
+    <div style={{ display: 'flex', flexDirection: 'column', alignItems: 'center', gap: 2, ...style }}>
+      <SpinnerButton onClick={onUp}>
+        <ChevronUp size={16} />
+      </SpinnerButton>
+      <div
+        style={{
+          height: 40,
+          display: 'flex',
+          alignItems: 'center',
+          justifyContent: 'center',
+          fontWeight: 700,
+          color: 'var(--text-primary)',
+          background: 'var(--bg-hover)',
+          borderRadius: 8,
+          ...valueStyle,
+        }}
+      >
+        {children}
+      </div>
+      <SpinnerButton onClick={onDown}>
+        <ChevronDown size={16} />
+      </SpinnerButton>
+    </div>
+  );
+}
+
+function SpinnerButton({ onClick, children }: { onClick: () => void; children: ReactNode }) {
+  return (
+    <button
+      type="button"
+      onClick={onClick}
+      style={spinnerButtonStyle}
+      onMouseEnter={(e) => (e.currentTarget.style.color = 'var(--text-primary)')}
+      onMouseLeave={(e) => (e.currentTarget.style.color = 'var(--text-faint)')}
+    >
+      {children}
+    </button>
+  );
+}
@@ -0,0 +1,13 @@
+import type { CSSProperties } from 'react';
+
+/** The look of the time picker's small chevron and clear buttons. */
+export const spinnerButtonStyle: CSSProperties = {
+  background: 'none',
+  border: 'none',
+  cursor: 'pointer',
+  padding: 2,
+  color: 'var(--text-faint)',
+  display: 'flex',
+  borderRadius: 4,
+  transition: 'color 0.15s',
+};