Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import AngleRightIcon from '@patternfly/react-icons/dist/esm/icons/angle-right-i
import { css } from '@patternfly/react-styles';
import styles from '@patternfly/react-styles/css/components/CalendarMonth/calendar-month';
import { getUniqueId } from '../../helpers/util';
import { isValidDate } from '../../helpers/datetimeUtils';

export enum Weekday {
Sunday = 0,
Expand Down Expand Up @@ -109,8 +110,6 @@ const buildCalendar = (year: number, month: number, weekStart: number, validator
const isSameDate = (d1: Date, d2: Date) =>
d1.getFullYear() === d2.getFullYear() && d1.getMonth() === d2.getMonth() && d1.getDate() === d2.getDate();

export const isValidDate = (date: Date) => Boolean(date && !isNaN(date as any));

const today = new Date();

/** The main calendar month component. */
Expand Down
3 changes: 2 additions & 1 deletion packages/react-core/src/components/DatePicker/DatePicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ import { TextInput, TextInputProps } from '../TextInput/TextInput';
import { Popover, PopoverProps } from '../Popover/Popover';
import { InputGroup } from '../InputGroup/InputGroup';
import OutlinedCalendarAltIcon from '@patternfly/react-icons/dist/esm/icons/outlined-calendar-alt-icon';
import { CalendarMonth, CalendarFormat, isValidDate } from '../CalendarMonth';
import { CalendarMonth, CalendarFormat } from '../CalendarMonth';
import { useImperativeHandle } from 'react';
import { KeyTypes } from '../../helpers';
import { isValidDate } from '../../helpers/datetimeUtils';

/** The main date picker component. */

Expand Down
31 changes: 26 additions & 5 deletions packages/react-core/src/components/Timestamp/Timestamp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as React from 'react';
import styles from '@patternfly/react-styles/css/components/Timestamp/timestamp';
import { css } from '@patternfly/react-styles';
import { Tooltip } from '../Tooltip';
import { isValidDate } from '../../helpers/datetimeUtils';

export enum TimestampFormat {
full = 'full',
Expand Down Expand Up @@ -78,7 +79,7 @@ export const Timestamp: React.FunctionComponent<TimestampProps> = ({
children,
className,
customFormat,
date: dateProp = new Date(),
date: dateProp,
dateFormat,
displaySuffix = '',
is12Hour,
Expand All @@ -87,34 +88,54 @@ export const Timestamp: React.FunctionComponent<TimestampProps> = ({
tooltip,
...props
}: TimestampProps) => {
const [date, setDate] = React.useState(() => {
const initDate = new Date(dateProp);
if (isValidDate(initDate)) {
return initDate;
}

return new Date();
});

React.useEffect(() => {
const dateFromProp = new Date(dateProp);
if (isValidDate(dateFromProp) && dateFromProp.toString() !== new Date(date).toString()) {
setDate(dateFromProp);
} else if (!dateProp) {
setDate(new Date());
}
}, [dateProp]);

const hasTimeFormat = timeFormat && !customFormat;
const formatOptions = {
...(dateFormat && !customFormat && { dateStyle: dateFormat }),
...(customFormat && { ...customFormat }),
...(is12Hour !== undefined && { hour12: is12Hour })
};

const dateAsLocaleString = new Date(dateProp).toLocaleString(locale, {
const dateAsLocaleString = new Date(date).toLocaleString(locale, {
...formatOptions,
...(hasTimeFormat && { timeStyle: timeFormat })
});
const defaultDisplay = `${dateAsLocaleString}${displaySuffix ? ' ' + displaySuffix : ''}`;

const utcTimeFormat = timeFormat !== 'short' ? 'medium' : 'short';
const convertToUTCString = (date: Date) => new Date(date).toUTCString().slice(0, -3);
const utcDateString = new Date(convertToUTCString(dateProp)).toLocaleString(locale, {
const utcDateString = new Date(convertToUTCString(date)).toLocaleString(locale, {
...formatOptions,
...(hasTimeFormat && { timeStyle: utcTimeFormat })
});
const defaultTooltipContent = `${utcDateString}${tooltip?.suffix ? ' ' + tooltip.suffix : ' UTC'}`;

const { dateTime, ...propsWithoutDateTime } = props;

const timestamp = (
<span
className={css(styles.timestamp, tooltip && styles.modifiers.helpText, className)}
{...(tooltip && { tabIndex: 0 })}
{...props}
{...propsWithoutDateTime}
>
<time className="pf-c-timestamp__text" dateTime={props.dateTime || new Date(dateProp).toISOString()}>
<time className="pf-c-timestamp__text" dateTime={dateTime || new Date(date).toISOString()}>
{!children ? defaultDisplay : children}
</time>
</span>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,31 @@ test('Renders with current date by default with default formatting', () => {
expect(screen.getByText(new Date().toLocaleString())).toBeInTheDocument();
});

test('Renders with correct datetime attribute with current date by default', () => {
render(<Timestamp />);
// Because there could be a .001 ms difference in the expected and received datetime value,
// we want an ISO value without the ms to expect as the datetime value.
const isoDateWithoutMS = new Date().toISOString().split('.')[0];

expect(screen.getByText(new Date().toLocaleString())).toHaveAttribute(
'datetime',
expect.stringMatching(isoDateWithoutMS)
);
});

test('Renders passed in date with default formatting', () => {
render(<Timestamp date={new Date(2022, 0, 1)} />);

expect(screen.getByText('1/1/2022, 12:00:00 AM')).toBeInTheDocument();
});

test('Renders with correct datetime attribute when date is passed in', () => {
const passedDate = new Date(2022, 0, 1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the user timezone going to affect these tests? @gitdallas, is there a way we can easily recognize the sorts of tests you had issues with before?

Copy link
Contributor

@gitdallas gitdallas Oct 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the test that fails for me without TZ=EST is "Renders with only date when dateFormat is passed in"
image

I think it is only happening when a timezone is provided (like EST in the above case) where it finds the user's time relative to that. That test will also pass for me if I use 01:00:00 for time, but not 00:59:59 (I'm 1 hour behind EST).

You could try it yourself by substituting '1 Jan 2022 00:00:00 EST' with '1 Jan 2022 23:59:00 PST'... it should show January 2nd.

In regard to new test in this PR - it does not have a TZ mentioned so it shouldn't have an issue in any case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe I had formatted the passed in string for those few tests due to the wonkiness that was occurring when running the tests (the tests would pass locally but fail when ran on Github).

I could update those few tests so that instead of new Date('...EST') the format is similar to other tests, e.g. new Date(2022, 0, 1). @gitdallas do you feel that would be a viable option?

render(<Timestamp date={passedDate} />);

expect(screen.getByText('1/1/2022, 12:00:00 AM')).toHaveAttribute('datetime', passedDate.toISOString());
});

test('Renders with custom formatting when dateFormat and timeFormat are passed in', () => {
render(
<Timestamp date={new Date(2022, 0, 1)} dateFormat={TimestampFormat.full} timeFormat={TimestampFormat.short} />
Expand All @@ -51,7 +70,7 @@ test('Renders with custom formatting when dateFormat and timeFormat are passed i
});

test('Renders with only date when dateFormat is passed in', () => {
render(<Timestamp date={new Date('1 Jan 2022 00:00:00 EST')} dateFormat={TimestampFormat.full} />);
render(<Timestamp date={new Date(2022, 0, 1)} dateFormat={TimestampFormat.full} />);

expect(screen.getByText('Saturday, January 1, 2022')).toBeInTheDocument();
});
Expand Down Expand Up @@ -152,17 +171,15 @@ test('Renders with pf-m-help-text class when tooltip is passed in with custom va
});

test('Renders with default tooltip content for default variant', () => {
render(
<Timestamp date={new Date('1 Jan 2022 00:00:00 EST')} tooltip={{ variant: TimestampTooltipVariant.default }} />
);
render(<Timestamp date={new Date(2022, 0, 1, 0, 0, 0)} tooltip={{ variant: TimestampTooltipVariant.default }} />);

expect(screen.getByText('1/1/2022, 5:00:00 AM UTC')).toBeInTheDocument();
});

test('Renders with custom tooltip suffix for default variant', () => {
render(
<Timestamp
date={new Date('1 Jan 2022 00:00:00 EST')}
date={new Date(2022, 0, 1, 0, 0, 0)}
tooltip={{ variant: TimestampTooltipVariant.default, suffix: 'Coordinated Universal Time' }}
/>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ exports[`Matches snapshot 1`] = `
<DocumentFragment>
<span
class="pf-c-timestamp"
datetime="2022-01-01T00:00:00.000Z"
>
<time
class="pf-c-timestamp__text"
Expand Down
4 changes: 4 additions & 0 deletions packages/react-core/src/helpers/datetimeUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/**
* @param {Date} date - A date to check the validity of
*/
export const isValidDate = (date: Date) => Boolean(date && !isNaN(date as any));
1 change: 1 addition & 0 deletions packages/react-core/src/helpers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ export * from './useIsomorphicLayout';
export * from './KeyboardHandler';
export * from './resizeObserver';
export * from './useInterval';
export * from './datetimeUtils';