Skip to content

Commit

Permalink
Code Review fixes
Browse files Browse the repository at this point in the history
- Remove invalid CSS rule
- Change TimePicker signature to return a Time object with hours and minutes,
  instead of a time string.
  • Loading branch information
nighto committed Dec 13, 2024
1 parent 5434d41 commit b248aa0
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
19 changes: 7 additions & 12 deletions src/components/forms/controls/DateTimePicker/DateTimePicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand All @@ -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>
);
Expand Down
20 changes: 14 additions & 6 deletions src/components/forms/controls/TimePicker/TimePicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,34 @@ 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,

/** 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',
Expand All @@ -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 },
Expand Down

0 comments on commit b248aa0

Please sign in to comment.