From 1132cb3ba66b7ce7d74a72efb6757edc7181d8e7 Mon Sep 17 00:00:00 2001
From: Alexander Polyankin <alexander.polyankin@metabase.com>
Date: Tue, 1 Mar 2022 13:57:35 +0300
Subject: [PATCH] Add 12-hour clock support to events (#20704)
---
.../widgets/FormDateWidget/FormDateWidget.tsx | 10 ++-
.../core/components/DateInput/DateInput.tsx | 33 ++++---
.../DateInput/DateInput.unit.spec.tsx | 12 ++-
.../components/DateSelector/DateSelector.tsx | 67 +++++++-------
.../core/components/DateWidget/DateWidget.tsx | 21 ++++-
.../TimeInput/TimeInput.stories.tsx | 9 +-
.../components/TimeInput/TimeInput.styled.tsx | 16 ++++
.../core/components/TimeInput/TimeInput.tsx | 87 ++++++++++++-------
.../TimeInput/TimeInput.unit.spec.tsx | 69 +++++++++++++--
.../entities/timeline-events/forms.js | 2 -
10 files changed, 236 insertions(+), 90 deletions(-)
diff --git a/frontend/src/metabase/components/form/widgets/FormDateWidget/FormDateWidget.tsx b/frontend/src/metabase/components/form/widgets/FormDateWidget/FormDateWidget.tsx
index 176f07d598e..3575d404e5d 100644
--- a/frontend/src/metabase/components/form/widgets/FormDateWidget/FormDateWidget.tsx
+++ b/frontend/src/metabase/components/form/widgets/FormDateWidget/FormDateWidget.tsx
@@ -1,6 +1,11 @@
import React, { forwardRef, Ref, useCallback, useMemo } from "react";
import { Moment } from "moment";
-import { parseTimestamp } from "metabase/lib/time";
+import {
+ getDateStyleFromSettings,
+ getTimeStyleFromSettings,
+ has24HourModeSetting,
+ parseTimestamp,
+} from "metabase/lib/time";
import DateWidget from "metabase/core/components/DateWidget";
import { FormField } from "./types";
@@ -49,6 +54,9 @@ const FormDateWidget = forwardRef(function FormDateWidget(
value={value}
placeholder={placeholder}
hasTime={hasTime}
+ dateFormat={getDateStyleFromSettings()}
+ timeFormat={getTimeStyleFromSettings()}
+ is24HourMode={has24HourModeSetting()}
readOnly={readOnly}
autoFocus={autoFocus}
error={field.visited && !field.active && field.error != null}
diff --git a/frontend/src/metabase/core/components/DateInput/DateInput.tsx b/frontend/src/metabase/core/components/DateInput/DateInput.tsx
index 5f1fb794ddd..fde066a2f4d 100644
--- a/frontend/src/metabase/core/components/DateInput/DateInput.tsx
+++ b/frontend/src/metabase/core/components/DateInput/DateInput.tsx
@@ -11,15 +11,12 @@ import React, {
} from "react";
import moment, { Moment } from "moment";
import { t } from "ttag";
-import {
- getDateStyleFromSettings,
- getTimeStyleFromSettings,
- hasTimePart,
-} from "metabase/lib/time";
+import { hasTimePart } from "metabase/lib/time";
import Input from "metabase/core/components/Input";
const DATE_FORMAT = "MM/DD/YYYY";
-const TIME_FORMAT = "HH:mm";
+const TIME_FORMAT_12 = "h:mm A";
+const TIME_FORMAT_24 = "HH:mm";
export type DateInputAttributes = Omit<
InputHTMLAttributes<HTMLDivElement>,
@@ -30,9 +27,11 @@ export interface DateInputProps extends DateInputAttributes {
value?: Moment;
inputRef?: Ref<HTMLInputElement>;
hasTime?: boolean;
+ hasCalendar?: boolean;
+ dateFormat?: string;
+ timeFormat?: string;
error?: boolean;
fullWidth?: boolean;
- hasCalendar?: boolean;
onChange?: (value?: Moment) => void;
onCalendarClick?: (event: MouseEvent<HTMLButtonElement>) => void;
}
@@ -43,9 +42,11 @@ const DateInput = forwardRef(function DateInput(
inputRef,
placeholder,
hasTime,
+ hasCalendar,
+ dateFormat = DATE_FORMAT,
+ timeFormat = TIME_FORMAT_12,
error,
fullWidth,
- hasCalendar,
onFocus,
onBlur,
onChange,
@@ -56,8 +57,6 @@ const DateInput = forwardRef(function DateInput(
) {
const [inputText, setInputText] = useState("");
const [isFocused, setIsFocused] = useState(false);
- const dateFormat = getDateStyleFromSettings() || DATE_FORMAT;
- const timeFormat = getTimeStyleFromSettings() || TIME_FORMAT;
const dateTimeFormat = `${dateFormat}, ${timeFormat}`;
const now = useMemo(() => {
@@ -78,6 +77,16 @@ const DateInput = forwardRef(function DateInput(
}
}, [value, hasTime, dateFormat, dateTimeFormat]);
+ const mixedTimeFormats = useMemo(
+ () => [
+ dateFormat,
+ dateTimeFormat,
+ `${dateFormat}, ${TIME_FORMAT_12}`,
+ `${dateFormat}, ${TIME_FORMAT_24}`,
+ ],
+ [dateFormat, dateTimeFormat],
+ );
+
const handleFocus = useCallback(
(event: FocusEvent<HTMLInputElement>) => {
setIsFocused(true);
@@ -100,7 +109,7 @@ const DateInput = forwardRef(function DateInput(
const newText = event.target.value;
setInputText(newText);
- const formats = hasTime ? [dateTimeFormat, dateFormat] : [dateFormat];
+ const formats = hasTime ? mixedTimeFormats : [dateFormat];
const newValue = moment(newText, formats);
if (newValue.isValid()) {
@@ -109,7 +118,7 @@ const DateInput = forwardRef(function DateInput(
onChange?.(undefined);
}
},
- [hasTime, dateFormat, dateTimeFormat, onChange],
+ [hasTime, dateFormat, mixedTimeFormats, onChange],
);
return (
diff --git a/frontend/src/metabase/core/components/DateInput/DateInput.unit.spec.tsx b/frontend/src/metabase/core/components/DateInput/DateInput.unit.spec.tsx
index 9b39125008a..a5d0a94a118 100644
--- a/frontend/src/metabase/core/components/DateInput/DateInput.unit.spec.tsx
+++ b/frontend/src/metabase/core/components/DateInput/DateInput.unit.spec.tsx
@@ -29,7 +29,17 @@ describe("DateInput", () => {
expect(onChange).toHaveBeenLastCalledWith(expected);
});
- it("should set date with time", () => {
+ it("should set date with time with 12-hour clock", () => {
+ const onChange = jest.fn();
+
+ render(<DateInputTest hasTime onChange={onChange} />);
+ userEvent.type(screen.getByRole("textbox"), "10/20/21 9:15 PM");
+
+ const expected = moment("10/20/21 9:15 PM", ["MM/DD/YYYY, h:mm A"]);
+ expect(onChange).toHaveBeenLastCalledWith(expected);
+ });
+
+ it("should set date with time with 24-hour clock", () => {
const onChange = jest.fn();
render(<DateInputTest hasTime onChange={onChange} />);
diff --git a/frontend/src/metabase/core/components/DateSelector/DateSelector.tsx b/frontend/src/metabase/core/components/DateSelector/DateSelector.tsx
index 4ce65a6d7cf..b91527f45b3 100644
--- a/frontend/src/metabase/core/components/DateSelector/DateSelector.tsx
+++ b/frontend/src/metabase/core/components/DateSelector/DateSelector.tsx
@@ -6,7 +6,7 @@ import React, {
useMemo,
useState,
} from "react";
-import moment, { Duration, Moment } from "moment";
+import moment, { Moment } from "moment";
import { t } from "ttag";
import { hasTimePart } from "metabase/lib/time";
import TimeInput from "metabase/core/components/TimeInput";
@@ -23,49 +23,53 @@ export interface DateSelectorProps {
style?: CSSProperties;
value?: Moment;
hasTime?: boolean;
+ is24HourMode?: boolean;
onChange?: (date?: Moment) => void;
onSubmit?: () => void;
}
const DateSelector = forwardRef(function DateSelector(
- { className, style, value, hasTime, onChange, onSubmit }: DateSelectorProps,
+ {
+ className,
+ style,
+ value,
+ hasTime,
+ is24HourMode,
+ onChange,
+ onSubmit,
+ }: DateSelectorProps,
ref: Ref<HTMLDivElement>,
): JSX.Element {
+ const today = useMemo(() => moment().startOf("date"), []);
const [isTimeShown, setIsTimeShown] = useState(hasTime && hasTimePart(value));
- const time = useMemo(() => {
- return moment.duration({
- hours: value?.hours(),
- minutes: value?.minutes(),
- });
- }, [value]);
-
const handleDateChange = useCallback(
- (unused1: string, unused2: string, dateStart: Moment) => {
- const newDate = dateStart.clone().local();
- newDate.hours(value ? value.hours() : 0);
- newDate.minutes(value ? value.minutes() : 0);
+ (unused1: string, unused2: string, date: Moment) => {
+ const newDate = moment({
+ year: date.year(),
+ month: date.month(),
+ day: date.day(),
+ hours: value?.hours(),
+ minutes: value?.minutes(),
+ });
onChange?.(newDate);
},
[value, onChange],
);
- const handleTimeChange = useCallback(
- (newTime?: Duration) => {
- const newDate = value ? value.clone() : moment().startOf("date");
- newDate.hours(newTime ? newTime.hours() : 0);
- newDate.minutes(newTime ? newTime.minutes() : 0);
- onChange?.(newDate);
- setIsTimeShown(newTime != null);
- },
- [value, onChange],
- );
-
const handleTimeClick = useCallback(() => {
- const newDate = value ? value.clone() : moment().startOf("date");
- onChange?.(newDate);
+ const newValue = value ?? today;
+ onChange?.(newValue);
setIsTimeShown(true);
- }, [value, onChange]);
+ }, [value, today, onChange]);
+
+ const handleTimeClear = useCallback(
+ (newValue: Moment) => {
+ onChange?.(newValue);
+ setIsTimeShown(false);
+ },
+ [onChange],
+ );
return (
<div ref={ref} className={className} style={style}>
@@ -75,9 +79,14 @@ const DateSelector = forwardRef(function DateSelector(
isRangePicker={false}
onChange={handleDateChange}
/>
- {isTimeShown && (
+ {value && isTimeShown && (
<SelectorTimeContainer>
- <TimeInput value={time} onChange={handleTimeChange} />
+ <TimeInput
+ value={value}
+ is24HourMode={is24HourMode}
+ onChange={onChange}
+ onClear={handleTimeClear}
+ />
</SelectorTimeContainer>
)}
<SelectorFooter>
diff --git a/frontend/src/metabase/core/components/DateWidget/DateWidget.tsx b/frontend/src/metabase/core/components/DateWidget/DateWidget.tsx
index 2104ba7554d..1445c66ed4e 100644
--- a/frontend/src/metabase/core/components/DateWidget/DateWidget.tsx
+++ b/frontend/src/metabase/core/components/DateWidget/DateWidget.tsx
@@ -18,13 +18,26 @@ export type DateWidgetAttributes = Omit<
export interface DateWidgetProps extends DateWidgetAttributes {
value?: Moment;
hasTime?: boolean;
+ dateFormat?: string;
+ timeFormat?: string;
+ is24HourMode?: boolean;
error?: boolean;
fullWidth?: boolean;
onChange?: (date?: Moment) => void;
}
const DateWidget = forwardRef(function DateWidget(
- { value, hasTime, error, fullWidth, onChange, ...props }: DateWidgetProps,
+ {
+ value,
+ hasTime,
+ dateFormat,
+ timeFormat,
+ is24HourMode,
+ error,
+ fullWidth,
+ onChange,
+ ...props
+ }: DateWidgetProps,
ref: Ref<HTMLDivElement>,
): JSX.Element {
const [isOpened, setIsOpened] = useState(false);
@@ -39,14 +52,14 @@ const DateWidget = forwardRef(function DateWidget(
return (
<TippyPopover
- trigger="manual"
- placement="bottom-start"
visible={isOpened}
+ placement="bottom-start"
interactive
content={
<DateSelector
value={value}
hasTime={hasTime}
+ is24HourMode={is24HourMode}
onChange={onChange}
onSubmit={handleClose}
/>
@@ -59,6 +72,8 @@ const DateWidget = forwardRef(function DateWidget(
value={value}
hasTime={hasTime}
hasCalendar={true}
+ dateFormat={dateFormat}
+ timeFormat={timeFormat}
error={error}
fullWidth={fullWidth}
onChange={onChange}
diff --git a/frontend/src/metabase/core/components/TimeInput/TimeInput.stories.tsx b/frontend/src/metabase/core/components/TimeInput/TimeInput.stories.tsx
index 33bacb11f39..d4bdf7e786a 100644
--- a/frontend/src/metabase/core/components/TimeInput/TimeInput.stories.tsx
+++ b/frontend/src/metabase/core/components/TimeInput/TimeInput.stories.tsx
@@ -1,5 +1,5 @@
import React, { useState } from "react";
-import { Duration } from "moment";
+import moment from "moment";
import { ComponentStory } from "@storybook/react";
import TimeInput from "./TimeInput";
@@ -9,8 +9,11 @@ export default {
};
const Template: ComponentStory<typeof TimeInput> = args => {
- const [value, setValue] = useState<Duration>();
- return <TimeInput {...args} value={value} onChange={setValue} />;
+ const [value, setValue] = useState(moment("2020-01-01T10:20"));
+
+ return (
+ <TimeInput {...args} value={value} onChange={setValue} onClear={setValue} />
+ );
};
export const Default = Template.bind({});
diff --git a/frontend/src/metabase/core/components/TimeInput/TimeInput.styled.tsx b/frontend/src/metabase/core/components/TimeInput/TimeInput.styled.tsx
index d6edfe07082..173821590d1 100644
--- a/frontend/src/metabase/core/components/TimeInput/TimeInput.styled.tsx
+++ b/frontend/src/metabase/core/components/TimeInput/TimeInput.styled.tsx
@@ -23,6 +23,22 @@ export const InputClearIcon = styled(Icon)`
color: ${color("text-light")};
`;
+interface InputPeriodButtonProps {
+ isSelected?: boolean;
+}
+
+export const InputMeridiemContainer = styled.div`
+ display: flex;
+ gap: 0.5rem;
+ margin-left: 0.5rem;
+`;
+
+export const InputMeridiemButton = styled.button<InputPeriodButtonProps>`
+ color: ${props => (props.isSelected ? color("brand") : color("text-light"))};
+ cursor: ${props => (props.isSelected ? "" : "pointer")};
+ font-weight: ${props => (props.isSelected ? "bold" : "")};
+`;
+
export const InputClearButton = styled(IconButtonWrapper)`
margin-left: auto;
`;
diff --git a/frontend/src/metabase/core/components/TimeInput/TimeInput.tsx b/frontend/src/metabase/core/components/TimeInput/TimeInput.tsx
index 7161a8279cf..e745a41f170 100644
--- a/frontend/src/metabase/core/components/TimeInput/TimeInput.tsx
+++ b/frontend/src/metabase/core/components/TimeInput/TimeInput.tsx
@@ -1,62 +1,87 @@
import React, { forwardRef, Ref, useCallback } from "react";
import { t } from "ttag";
-import moment, { Duration } from "moment";
+import moment, { Moment } from "moment";
import Tooltip from "metabase/components/Tooltip";
import {
InputClearButton,
InputClearIcon,
InputDivider,
InputField,
+ InputMeridiemButton,
+ InputMeridiemContainer,
InputRoot,
} from "./TimeInput.styled";
-const HOURS_MAX = 24;
-const MINUTES_MAX = 60;
-
export interface TimeInputProps {
- value?: Duration;
+ value: Moment;
+ is24HourMode?: boolean;
autoFocus?: boolean;
- onChange?: (value?: Duration) => void;
+ onChange?: (value: Moment) => void;
+ onClear?: (value: Moment) => void;
}
const TimeInput = forwardRef(function TimeInput(
- { value, onChange }: TimeInputProps,
+ { value, is24HourMode, autoFocus, onChange, onClear }: TimeInputProps,
ref: Ref<HTMLDivElement>,
): JSX.Element {
- const hoursText = formatTime(value?.hours());
- const minutesText = formatTime(value?.minutes());
+ const hoursText = value.format(is24HourMode ? "HH" : "hh");
+ const minutesText = value.format("mm");
+ const isAm = value.hours() < 12;
+ const isPm = !isAm;
+ const amText = moment.localeData().meridiem(0, 0, false);
+ const pmText = moment.localeData().meridiem(12, 0, false);
const handleHoursChange = useCallback(
- (hours?: number) => {
- const newValue = moment.duration({
- hours: hours ? hours % HOURS_MAX : 0,
- minutes: value ? value.minutes() : 0,
- });
+ (hours = 0) => {
+ const newValue = value.clone();
+ if (is24HourMode) {
+ newValue.hours(hours % 24);
+ } else {
+ newValue.hours((hours % 12) + (isAm ? 0 : 12));
+ }
onChange?.(newValue);
},
- [value, onChange],
+ [value, isAm, is24HourMode, onChange],
);
const handleMinutesChange = useCallback(
- (minutes?: number) => {
- const newValue = moment.duration({
- hours: value ? value.hours() : 0,
- minutes: minutes ? minutes % MINUTES_MAX : 0,
- });
+ (minutes = 0) => {
+ const newValue = value.clone();
+ newValue.minutes(minutes % 60);
onChange?.(newValue);
},
[value, onChange],
);
+ const handleAmClick = useCallback(() => {
+ if (isPm) {
+ const newValue = value.clone();
+ newValue.hours(newValue.hours() - 12);
+ onChange?.(newValue);
+ }
+ }, [value, isPm, onChange]);
+
+ const handlePmClick = useCallback(() => {
+ if (isAm) {
+ const newValue = value.clone();
+ newValue.hours(newValue.hours() + 12);
+ onChange?.(newValue);
+ }
+ }, [value, isAm, onChange]);
+
const handleClearClick = useCallback(() => {
- onChange?.(undefined);
- }, [onChange]);
+ const newValue = value.clone();
+ newValue.hours(0);
+ newValue.minutes(0);
+ onClear?.(newValue);
+ }, [value, onClear]);
return (
<InputRoot ref={ref}>
<InputField
value={hoursText}
placeholder="00"
+ autoFocus={autoFocus}
fullWidth
aria-label={t`Hours`}
onChange={handleHoursChange}
@@ -69,6 +94,16 @@ const TimeInput = forwardRef(function TimeInput(
aria-label={t`Minutes`}
onChange={handleMinutesChange}
/>
+ {!is24HourMode && (
+ <InputMeridiemContainer>
+ <InputMeridiemButton isSelected={isAm} onClick={handleAmClick}>
+ {amText}
+ </InputMeridiemButton>
+ <InputMeridiemButton isSelected={isPm} onClick={handlePmClick}>
+ {pmText}
+ </InputMeridiemButton>
+ </InputMeridiemContainer>
+ )}
<Tooltip tooltip={t`Remove time`}>
<InputClearButton
aria-label={t`Remove time`}
@@ -81,12 +116,4 @@ const TimeInput = forwardRef(function TimeInput(
);
});
-const formatTime = (value?: number) => {
- if (value != null) {
- return value < 10 ? `0${value}` : `${value}`;
- } else {
- return "";
- }
-};
-
export default TimeInput;
diff --git a/frontend/src/metabase/core/components/TimeInput/TimeInput.unit.spec.tsx b/frontend/src/metabase/core/components/TimeInput/TimeInput.unit.spec.tsx
index 77f9ed2c752..f4b12bb4c42 100644
--- a/frontend/src/metabase/core/components/TimeInput/TimeInput.unit.spec.tsx
+++ b/frontend/src/metabase/core/components/TimeInput/TimeInput.unit.spec.tsx
@@ -1,14 +1,14 @@
import React, { useCallback, useState } from "react";
-import { duration, Duration } from "moment";
+import moment, { Moment } from "moment";
import { render, screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import TimeInput, { TimeInputProps } from "./TimeInput";
const TestTimeInput = ({ onChange, ...props }: TimeInputProps) => {
- const [value, setValue] = useState<Duration>();
+ const [value, setValue] = useState(props.value);
const handleChange = useCallback(
- (value?: Duration) => {
+ (value: Moment) => {
setValue(value);
onChange?.(value);
},
@@ -19,25 +19,76 @@ const TestTimeInput = ({ onChange, ...props }: TimeInputProps) => {
};
describe("TimeInput", () => {
- it("should set time", () => {
+ it("should set time in 12-hour clock", () => {
+ const value = moment({ hours: 0, minutes: 0 });
const onChange = jest.fn();
- render(<TestTimeInput onChange={onChange} />);
+ render(<TestTimeInput value={value} onChange={onChange} />);
+ userEvent.clear(screen.getByLabelText("Hours"));
userEvent.type(screen.getByLabelText("Hours"), "5");
+ userEvent.clear(screen.getByLabelText("Minutes"));
userEvent.type(screen.getByLabelText("Minutes"), "20");
- const expected = duration({ hours: 5, minutes: 20 });
+ const expected = value.clone();
+ expected.hours(5);
+ expected.minutes(20);
expect(onChange).toHaveBeenLastCalledWith(expected);
});
- it("should clear time", () => {
+ it("should set time in 24-hour clock", () => {
+ const value = moment({ hours: 0, minutes: 0 });
const onChange = jest.fn();
- render(<TestTimeInput onChange={onChange} />);
+ render(<TestTimeInput value={value} is24HourMode onChange={onChange} />);
+ userEvent.clear(screen.getByLabelText("Hours"));
+ userEvent.type(screen.getByLabelText("Hours"), "15");
+ userEvent.clear(screen.getByLabelText("Minutes"));
+ userEvent.type(screen.getByLabelText("Minutes"), "10");
+
+ const expected = value.clone();
+ expected.hours(15);
+ expected.minutes(10);
+ expect(onChange).toHaveBeenLastCalledWith(expected);
+ });
+
+ it("should change meridiem to am", () => {
+ const value = moment({ hours: 12, minutes: 20 });
+ const onChange = jest.fn();
+
+ render(<TestTimeInput value={value} onChange={onChange} />);
+ userEvent.click(screen.getByText("AM"));
+
+ const expected = value.clone();
+ expected.hours(0);
+ expect(onChange).toHaveBeenCalledWith(expected);
+ });
+
+ it("should change meridiem to pm", () => {
+ const value = moment({ hours: 10, minutes: 20 });
+ const onChange = jest.fn();
+
+ render(<TestTimeInput value={value} onChange={onChange} />);
+ userEvent.click(screen.getByText("PM"));
+
+ const expected = value.clone();
+ expected.hours(22);
+ expect(onChange).toHaveBeenCalledWith(expected);
+ });
+
+ it("should clear time", () => {
+ const value = moment({ hours: 2, minutes: 10 });
+ const onClear = jest.fn();
+
+ render(<TestTimeInput value={value} onClear={onClear} />);
+ userEvent.clear(screen.getByLabelText("Hours"));
userEvent.type(screen.getByLabelText("Hours"), "5");
+ userEvent.clear(screen.getByLabelText("Minutes"));
userEvent.type(screen.getByLabelText("Minutes"), "20");
userEvent.click(screen.getByLabelText("Remove time"));
- expect(onChange).toHaveBeenLastCalledWith(undefined);
+ const expected = value.clone();
+ expected.hours(0);
+ expected.minutes(0);
+ expect(onClear).toHaveBeenCalledWith(expected);
});
});
diff --git a/frontend/src/metabase/entities/timeline-events/forms.js b/frontend/src/metabase/entities/timeline-events/forms.js
index 63ad67c70c3..97cefda51f0 100644
--- a/frontend/src/metabase/entities/timeline-events/forms.js
+++ b/frontend/src/metabase/entities/timeline-events/forms.js
@@ -17,8 +17,6 @@ const createForm = () => {
title: t`Date`,
type: "date",
hasTime: true,
- hasTimezone: true,
- timezoneFieldName: "timezone",
validate: validate.required(),
},
{
--
GitLab