Skip to content

Commit

Permalink
DateTimePicker: address feedback after recent refactor to `date-fns…
Browse files Browse the repository at this point in the history
…` and `use-lilius` (#43495)

* Use compiler-time cast instead of run-time cast

* Add box-sizing: border-box

* Prevent scroll on arrow press

* Implement PageUp, PageDown, Home and End keyboard shortcuts

* Set DayOfWeek font-size

* Use advanceTimersByTime

* Add line-height

* Add comment explaining type cast

* Add exhaustive-deps ignore

* Update CHANGELOG.md
  • Loading branch information
noisysocks authored Aug 24, 2022
1 parent 1a108a4 commit 9e80dab
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 13 deletions.
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
- Update `floating-ui` to the latest version ([#43206](https://github.com/WordPress/gutenberg/pull/43206)).
- `DateTimePicker`, `TimePicker`, `DatePicker`: Switch from `moment` to `date-fns` ([#43005](https://github.com/WordPress/gutenberg/pull/43005)).
- `DatePicker`: Switch from `react-dates` to `use-lilius` ([#43005](https://github.com/WordPress/gutenberg/pull/43005)).
- `DateTimePicker`: address feedback after recent refactor to `date-fns` and `use-lilius` ([#43495](https://github.com/WordPress/gutenberg/pull/43495)).
- `convertLTRToRTL()`: Refactor away from `_.mapKeys()` ([#43258](https://github.com/WordPress/gutenberg/pull/43258/)).
- `withSpokenMessages`: Update to use `@testing-library/react` ([#43273](https://github.com/WordPress/gutenberg/pull/43273)).
- `MenuGroup`: Refactor unit tests to use `@testing-library/react` ([#43275](https://github.com/WordPress/gutenberg/pull/43275)).
Expand Down
7 changes: 3 additions & 4 deletions packages/components/src/date-time/date-time/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,10 @@ import Button from '../../button';
import { default as DatePicker } from '../date';
import { default as TimePicker } from '../time';
import type { DateTimePickerProps } from '../types';
import { CalendarHelp } from './styles';
import { Wrapper, CalendarHelp } from './styles';
import { HStack } from '../../h-stack';
import { Heading } from '../../heading';
import { Spacer } from '../../spacer';
import { VStack } from '../../v-stack';

export { DatePicker, TimePicker };

Expand Down Expand Up @@ -64,7 +63,7 @@ function UnforwardedDateTimePicker(
}

return (
<VStack ref={ ref } className="components-datetime" spacing={ 4 }>
<Wrapper ref={ ref } className="components-datetime" spacing={ 4 }>
{ ! calendarHelpIsVisible && (
<>
<TimePicker
Expand Down Expand Up @@ -187,7 +186,7 @@ function UnforwardedDateTimePicker(
) }
</HStack>
) }
</VStack>
</Wrapper>
);
}

Expand Down
9 changes: 9 additions & 0 deletions packages/components/src/date-time/date-time/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,15 @@
*/
import styled from '@emotion/styled';

/**
* Internal dependencies
*/
import { VStack } from '../../v-stack';

export const Wrapper = styled( VStack )`
box-sizing: border-box;
`;

export const CalendarHelp = styled.div`
min-width: 260px;
`;
32 changes: 26 additions & 6 deletions packages/components/src/date-time/date/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import {
subWeeks,
addWeeks,
isSameMonth,
startOfWeek,
endOfWeek,
} from 'date-fns';

/**
Expand All @@ -28,6 +30,7 @@ import { useState, useRef, useEffect } from '@wordpress/element';
*/
import type { DatePickerProps } from '../types';
import {
Wrapper,
Navigator,
NavigatorHeading,
Calendar,
Expand Down Expand Up @@ -63,7 +66,7 @@ export function DatePicker( {
events = [],
isInvalidDate,
onMonthPreviewed,
startOfWeek = 0,
startOfWeek: weekStartsOn = 0,
}: DatePickerProps ) {
const date = currentDate ? inputToDate( currentDate ) : new Date();

Expand All @@ -78,7 +81,7 @@ export function DatePicker( {
} = useLilius( {
selected: [ startOfDay( date ) ],
viewing: startOfDay( date ),
weekStartsOn: startOfWeek,
weekStartsOn,
} );

// Used to implement a roving tab index. Tracks the day that receives focus
Expand All @@ -101,7 +104,7 @@ export function DatePicker( {
}

return (
<div
<Wrapper
className="components-datetime__date"
role="application"
aria-label={ __( 'Calendar' ) }
Expand Down Expand Up @@ -218,7 +221,22 @@ export function DatePicker( {
if ( event.key === 'ArrowDown' ) {
nextFocusable = addWeeks( day, 1 );
}
if ( event.key === 'PageUp' ) {
nextFocusable = subMonths( day, 1 );
}
if ( event.key === 'PageDown' ) {
nextFocusable = addMonths( day, 1 );
}
if ( event.key === 'Home' ) {
nextFocusable = startOfWeek( day );
}
if ( event.key === 'End' ) {
nextFocusable = startOfDay(
endOfWeek( day )
);
}
if ( nextFocusable ) {
event.preventDefault();
setFocusable( nextFocusable );
if (
! isSameMonth(
Expand All @@ -241,7 +259,7 @@ export function DatePicker( {
} )
) }
</Calendar>
</div>
</Wrapper>
);
}

Expand Down Expand Up @@ -274,12 +292,14 @@ function Day( {

// Focus the day when it becomes focusable, e.g. because an arrow key is
// pressed. Only do this if focus is allowed - this stops us stealing focus
// from e.g. a TimePicker input. Note that isFocusAllowed is not a dep as
// there is no point calling focus() on an already focused element.
// from e.g. a TimePicker input.
useEffect( () => {
if ( ref.current && isFocusable && isFocusAllowed ) {
ref.current.focus();
}
// isFocusAllowed is not a dep as there is no point calling focus() on
// an already focused element.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [ isFocusable ] );

return (
Expand Down
6 changes: 6 additions & 0 deletions packages/components/src/date-time/date/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ import { HStack } from '../../h-stack';
import { Heading } from '../../heading';
import { space } from '../../ui/utils/space';

export const Wrapper = styled.div`
box-sizing: border-box;
`;

export const Navigator = styled( HStack )`
margin-bottom: ${ space( 4 ) };
`;
Expand All @@ -35,6 +39,8 @@ export const Calendar = styled.div`

export const DayOfWeek = styled.div`
color: ${ COLORS.gray[ 700 ] };
font-size: ${ CONFIG.fontSize };
line-height: ${ CONFIG.fontLineHeightBase };
&:nth-of-type( 1 ) {
justify-self: start;
Expand Down
8 changes: 6 additions & 2 deletions packages/components/src/date-time/date/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ describe( 'DatePicker', () => {
} );

it( 'should call onChange when a day is selected', async () => {
const user = userEvent.setup( { delay: null } );
const user = userEvent.setup( {
advanceTimers: jest.advanceTimersByTime,
} );

const onChange = jest.fn();

Expand All @@ -50,7 +52,9 @@ describe( 'DatePicker', () => {
} );

it( 'should call onMonthPreviewed and onChange when a day in a different month is selected', async () => {
const user = userEvent.setup( { delay: null } );
const user = userEvent.setup( {
advanceTimers: jest.advanceTimersByTime,
} );

const onMonthPreviewed = jest.fn();
const onChange = jest.fn();
Expand Down
1 change: 1 addition & 0 deletions packages/components/src/date-time/time/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import SelectControl from '../../select-control';
import { Select } from '../../select-control/styles/select-control-styles';

export const Wrapper = styled.div`
box-sizing: border-box;
font-size: ${ CONFIG.fontSize };
`;

Expand Down
3 changes: 2 additions & 1 deletion packages/date/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,8 @@ function buildMoment( dateValue, timezone = '' ) {
const dateMoment = momentLib( dateValue );

if ( timezone && ! isUTCOffset( timezone ) ) {
return dateMoment.tz( String( timezone ) );
// The ! isUTCOffset() check guarantees that timezone is a string.
return dateMoment.tz( /** @type {string} */ ( timezone ) );
}

if ( timezone && isUTCOffset( timezone ) ) {
Expand Down

0 comments on commit 9e80dab

Please sign in to comment.