Various code improvements and added keyboard shortcuts (#10)

* various code improvements and added keyboard shortcuts

* update tests

* fix time input field

* fix time input better

Author: mimiflynn

package.json

@@ -27,6 +27,9 @@
   "type": "module",
   "jest": {
     "testEnvironment": "jsdom",
+    "setupFilesAfterEnv": [
+      "<rootDir>/src/setupTests.js"
+    ],
     "moduleNameMapper": {
       "^.+\\.svg$": "jest-svg-transformer",
       "^.+\\.(css|less|scss)$": "identity-obj-proxy"

src/App.jsx

@@ -1,4 +1,4 @@
-import React, { useCallback, useEffect, useMemo, useState } from 'react';
+import React, { useCallback, useEffect, useState } from 'react';
 import classNames from 'classnames';
 import './styles/App.css';
 import { DEFAULT_LIMIT_SEC, DEFAULT_WARNING_SEC } from './constants';
@@ -12,15 +12,30 @@ const App = () => {
   const [timeWarning, setTimeWarning] = useState(DEFAULT_WARNING_SEC);
 
   useEffect(() => {
-    if (timerStarted) {
-      const timeout = setTimeout(() => {
-        setTime(timer(timeElapsed));
-      }, 1000);
-      return () => clearTimeout(timeout);
-    } else {
-      setTime(timeElapsed);
-    }
-  }, [setTime, timeElapsed, timerStarted]);
+    if (!timerStarted) return;
+
+    const timeout = setTimeout(() => {
+      setTime(timer(timeElapsed));
+    }, 1000);
+
+    return () => clearTimeout(timeout);
+  }, [timeElapsed, timerStarted]);
+
+  // Handle keyboard shortcuts
+  useEffect(() => {
+    const handleKeyPress = (event) => {
+      if (event.code === 'Space') {
+        event.preventDefault();
+        setTimerStatus((prev) => !prev);
+      } else if (event.code === 'KeyR') {
+        event.preventDefault();
+        setTime(0);
+      }
+    };
+
+    window.addEventListener('keydown', handleKeyPress);
+    return () => window.removeEventListener('keydown', handleKeyPress);
+  }, []);
 
   const handleLimitUpdate = useCallback((value) => {
     setTimeLimit(value);
@@ -30,73 +45,97 @@ const App = () => {
     setTimeWarning(value);
   }, []);
 
-  const renderControls = useMemo(() => {
-    return (
-      <div>
-        <button
-          type="button"
-          className={classNames('btn', {
-            'btn-primary': !timerStarted,
-            'btn-danger': timerStarted,
-          })}
-          onClick={() => setTimerStatus(!timerStarted)}
-        >
-          {timerStarted ? 'Stop' : 'Start'}
-        </button>
-        <button
-          type="button"
-          className="btn btn-dark"
-          onClick={() => setTime(0)}
-        >
-          Reset
-        </button>
+  const handleReset = useCallback(() => {
+    setTime(0);
+  }, []);
+
+  const handleToggleTimer = useCallback(() => {
+    setTimerStatus((prev) => !prev);
+  }, []);
+
+  const isWarning = timeElapsed > timeLimit - timeWarning;
+  const isOverLimit = timeElapsed > timeLimit;
+
+  return (
+    <div className="position-relative">
+      <div
+        className={classNames('timer', {
+          warning: isWarning && !isOverLimit,
+          stop: isOverLimit,
+        })}
+      >
+        <div className="time" role="timer" aria-live="polite">
+          {formatTime(timeElapsed)}
+        </div>
+
+        <div>
+          <button
+            type="button"
+            className={classNames('btn', {
+              'btn-primary': !timerStarted,
+              'btn-danger': timerStarted,
+            })}
+            onClick={handleToggleTimer}
+            aria-label={timerStarted ? 'Stop timer' : 'Start timer'}
+          >
+            {timerStarted ? 'Stop' : 'Start'}
+          </button>
+          <button
+            type="button"
+            className="btn btn-dark"
+            onClick={handleReset}
+            aria-label="Reset timer to zero"
+          >
+            Reset
+          </button>
+        </div>
       </div>
-    );
-  }, [timerStarted]);
 
-  const renderSettings = useMemo(() => {
-    return (
       <div className="container my-5">
         <div className="row">
           <div className="col">
             <div className="input-group">
-              <label className="input-group-text">Limit</label>
+              <label className="input-group-text" htmlFor="time-limit-input">
+                Limit
+              </label>
               <TimeInput
-                ariaLabel="Set Time Limit"
-                ariaDescribedby="time-limit"
+                id="time-limit-input"
+                ariaLabel="Set time limit in minutes and seconds"
+                ariaDescribedby="time-limit-help"
                 placeholderSec={timeLimit}
                 onChange={handleLimitUpdate}
-              ></TimeInput>
+              />
             </div>
+            <small id="time-limit-help" className="form-text text-muted">
+              Timer stops when limit is reached
+            </small>
           </div>
           <div className="col">
             <div className="input-group">
               <TimeInput
-                ariaLabel="Set Time Limit"
-                ariaDescribedby="time-limit"
+                id="time-warning-input"
+                ariaLabel="Set warning time in minutes and seconds"
+                ariaDescribedby="time-warning-help"
                 placeholderSec={timeWarning}
                 onChange={handleWarningUpdate}
-              ></TimeInput>
-              <label className="input-group-text">Warning</label>
+              />
+              <label className="input-group-text" htmlFor="time-warning-input">
+                Warning
+              </label>
             </div>
+            <small id="time-warning-help" className="form-text text-muted">
+              Warning shown before time limit
+            </small>
+          </div>
+        </div>
+        <div className="row mt-3">
+          <div className="col text-center">
+            <small className="text-muted">
+              Keyboard shortcuts: Space to start/stop, R to reset
+            </small>
           </div>
         </div>
       </div>
-    );
-  }, [handleLimitUpdate, handleWarningUpdate, timeLimit, timeWarning]);
-
-  return (
-    <div className="position-relative">
-      <div
-        className={classNames('timer', {
-          warning: timeElapsed > timeLimit - timeWarning,
-          stop: timeElapsed > timeLimit,
-        })}
-      >
-        <div className="time">{formatTime(timeElapsed)}</div>
-        {renderControls}
-      </div>
-      {renderSettings}
     </div>
   );
 };

src/components/time-input.jsx

@@ -12,13 +12,26 @@ export const TimeInput = ({
   onChange,
 }) => {
   const [value, setValue] = useState(formatTime(placeholderSec));
-  const handleChange = useCallback(
-    ({ target }) => {
-      setValue(formatInputTime(formatTime(target.value)));
-      onChange(convertToSeconds(target.value));
-    },
-    [onChange]
-  );
+  const [isEditing, setIsEditing] = useState(false);
+
+  const handleChange = useCallback(({ target }) => {
+    // While editing, just store the raw value (digits only)
+    setValue(target.value);
+  }, []);
+
+  const handleFocus = useCallback(() => {
+    setIsEditing(true);
+    // Clear the formatted value to let user type fresh
+    setValue('');
+  }, []);
+
+  const handleBlur = useCallback(() => {
+    setIsEditing(false);
+    // Format the value when done editing
+    const formattedValue = formatInputTime(value);
+    setValue(formattedValue);
+    onChange(convertToSeconds(formattedValue));
+  }, [value, onChange]);
 
   return (
     <input
@@ -29,6 +42,8 @@ export const TimeInput = ({
       aria-describedby={ariaDescribedby}
       placeholder={formatTime(placeholderSec)}
       onChange={handleChange}
+      onFocus={handleFocus}
+      onBlur={handleBlur}
       value={value}
     ></input>
   );

src/components/time-input.test.js

@@ -7,10 +7,10 @@ const setup = () => {
     <TimeInput
       ariaLabel="time"
       ariaDescribedby="time-limit"
-      placeholder="0:30"
+      placeholder="2:30"
       onChange={(event) => console.log('change', event)}
       value={90}
-    ></TimeInput>
+    ></TimeInput>,
   );
   const input = screen.getByLabelText('time');
   return {
@@ -21,18 +21,25 @@ const setup = () => {
 
 test('Input should set display value', () => {
   const { input } = setup();
+
+  // Focus, type, then blur to trigger formatting
+  fireEvent.focus(input);
   fireEvent.change(input, { target: { value: '23' } });
+  fireEvent.blur(input);
   expect(input.value).toBe('0:23');
 
+  fireEvent.focus(input);
   fireEvent.change(input, { target: { value: 'y15efg' } });
+  fireEvent.blur(input);
   expect(input.value).toBe('0:15');
 
-  fireEvent.change(input, { target: { value: '8' } });
-  fireEvent.change(input, { target: { value: '80' } });
+  fireEvent.focus(input);
   fireEvent.change(input, { target: { value: '800' } });
+  fireEvent.blur(input);
   expect(input.value).toBe('8:00');
 
-  fireEvent.change(input, { target: { value: '9' } });
+  fireEvent.focus(input);
   fireEvent.change(input, { target: { value: '90' } });
+  fireEvent.blur(input);
   expect(input.value).toBe('1:30');
 });