From b248aa06c1b5616bb1dadfa71167ad77f4759f58 Mon Sep 17 00:00:00 2001
From: Arlindo Pereira <arlindosaraivapereira@gmail.com>
Date: Fri, 13 Dec 2024 11:30:10 +0100
Subject: [PATCH] Code Review fixes

- Remove invalid CSS rule
- Change TimePicker signature to return a Time object with hours and minutes,
  instead of a time string.
---
 .../DatePicker/DatePicker.module.scss         |  6 ------
 .../DateTimePicker/DateTimePicker.tsx         | 19 +++++++-----------
 .../TimePicker/TimePicker.stories.tsx         |  6 +++---
 .../forms/controls/TimePicker/TimePicker.tsx  | 20 +++++++++++++------
 4 files changed, 24 insertions(+), 27 deletions(-)

diff --git a/src/components/forms/controls/DatePicker/DatePicker.module.scss b/src/components/forms/controls/DatePicker/DatePicker.module.scss
index 8daaf74..7cedf3a 100644
--- a/src/components/forms/controls/DatePicker/DatePicker.module.scss
+++ b/src/components/forms/controls/DatePicker/DatePicker.module.scss
@@ -155,12 +155,6 @@ $navigation-distance: 1rem;
   border-radius: bk.$border-radius-circle;
 }
 
-/* stylelint-disable-next-line */
-:global(.react-datepicker__day:not(&--selected):hover) {
-  background: rgba(bk.$color-blueberry-500, 0.5); // TODO use proper variable
-  border-radius: 50%;
-}
-
 :global(.react-datepicker-popper[data-placement^="bottom"]) {
   margin-top: 0;
 }
diff --git a/src/components/forms/controls/DateTimePicker/DateTimePicker.tsx b/src/components/forms/controls/DateTimePicker/DateTimePicker.tsx
index cce6d9f..4daa251 100644
--- a/src/components/forms/controls/DateTimePicker/DateTimePicker.tsx
+++ b/src/components/forms/controls/DateTimePicker/DateTimePicker.tsx
@@ -7,7 +7,7 @@ import * as React from 'react';
 import { classNames as cx, type ClassNameArgument } from '../../../../util/componentUtil.ts';
 
 import { DatePicker } from '../DatePicker/DatePicker.tsx';
-import { TimePicker } from '../TimePicker/TimePicker.tsx';
+import { TimePicker, type Time } from '../TimePicker/TimePicker.tsx';
 
 import cl from './DateTimePicker.module.scss';
 
@@ -29,23 +29,18 @@ type GenericProps = {
 export type DateTimePickerProps = GenericProps;
 
 export const DateTimePicker = ({ unstyled = false, className, date, onChange, ...propsRest }: DateTimePickerProps) => {
-  // time string from date object
-  const time = `${String(date?.getHours() || 0).padStart(2, '0')}:${String(date?.getMinutes() || 0).padStart(2, '0')}`;
+  // time from date object
+  const time = { hours: date?.getHours() || 0, minutes: date?.getMinutes() || 0 };
   
   // manually update upstream date object when time is updated
-  const onTimeUpdate = (time: string) => {
+  const onTimeUpdate = (time: Time) => {
     if (date) {
       const newDate = new Date(date);
-      const [newHours, newMinutes] = time.split(':');
-      if (newHours) {
-        newDate.setHours(Number(newHours));
-      }
-      if (newMinutes) {
-        newDate.setMinutes(Number(newMinutes));
-      }
+      newDate.setHours(time.hours);
+      newDate.setMinutes(time.minutes);
       onChange(newDate);
     }
-  }
+  };
 
   return (
     <div className={cx(
diff --git a/src/components/forms/controls/TimePicker/TimePicker.stories.tsx b/src/components/forms/controls/TimePicker/TimePicker.stories.tsx
index 51d855d..4d3a078 100644
--- a/src/components/forms/controls/TimePicker/TimePicker.stories.tsx
+++ b/src/components/forms/controls/TimePicker/TimePicker.stories.tsx
@@ -6,7 +6,7 @@ import type { Meta, StoryObj } from '@storybook/react';
 
 import * as React from 'react';
 
-import { TimePicker } from './TimePicker.tsx';
+import { TimePicker, type Time } from './TimePicker.tsx';
 
 
 type TimePickerArgs = React.ComponentProps<typeof TimePicker>;
@@ -29,13 +29,13 @@ export default {
 export const TimePickerStory: Story = {
   name: 'Time Picker',
   render: (args) => {
-    const [time, setTime] = React.useState<string>('09:41');
+    const [time, setTime] = React.useState<Time>({ hours: 9, minutes: 41 });
 
     return (
       <div>
         <TimePicker time={time} onUpdate={setTime}/>
         <p>
-          The selected time is: {time}
+          The selected time is: {time.hours}:{time.minutes}
         </p>
       </div>
     );
diff --git a/src/components/forms/controls/TimePicker/TimePicker.tsx b/src/components/forms/controls/TimePicker/TimePicker.tsx
index 9f296d9..f611e9e 100644
--- a/src/components/forms/controls/TimePicker/TimePicker.tsx
+++ b/src/components/forms/controls/TimePicker/TimePicker.tsx
@@ -11,6 +11,11 @@ import { Input } from '../Input/Input.tsx';
 import cl from './TimePicker.module.scss';
 
 
+export type Time = {
+  hours: number,
+  minutes: number,
+}
+
 export type TimePickerProps = ComponentProps<'input'> & {
   /** Whether this component should be unstyled. */
   unstyled?: undefined | boolean,
@@ -18,19 +23,22 @@ export type TimePickerProps = ComponentProps<'input'> & {
   /** An optional class name to be appended to the class list. */
   className?: ClassNameArgument,
   
-  /** A time string as defined to be used by input type="time", with the hh:mm format. */
-  time: string,
+  /** A time object with hours and minutes, as numbers. */
+  time: Time,
   
   /** A callback function to update the time. */
-  onUpdate: (time: string) => void,
+  onUpdate: (time: Time) => void,
 };
 
 export const TimePicker = ({ unstyled = false, className, time, onUpdate, ...propsRest }: TimePickerProps) => {
+  const timeString = `${String(time.hours).padStart(2, '0')}:${String(time.minutes).padStart(2, '0')}`;
+
   const onChange = (e: React.ChangeEvent<HTMLInputElement>) => {
     const newTimeString = e.target.value;
-    onUpdate(newTimeString);
+    const [hours, minutes] = newTimeString.split(':');
+    onUpdate({ hours: Number(hours), minutes: Number(minutes) });
   };
-  
+
   return (
     <div className={cx(
       'bk',
@@ -39,7 +47,7 @@ export const TimePicker = ({ unstyled = false, className, time, onUpdate, ...pro
     )}>
       <Input
         type="time"
-        value={time}
+        value={timeString}
         onChange={onChange}
         className={cx(
           { [cl['bk-timepicker--input']]: !unstyled },