-
Notifications
You must be signed in to change notification settings - Fork 4.6k
DateTime: Create TimeInput component and integrate into TimePicker #60613
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
5351786
Move reducer util func to the upper level of utils
bogiii 9d33a34
Move from12hTo24h util func to the upper level of utils
bogiii 745d365
Extract validation logic into separate function
bogiii 121f3bb
Add from24hTo12h util method
bogiii ec654b4
Create initial version of TimeInput component
bogiii 77ed54c
Support two way data binding of the hours and minutes props
bogiii b6c037f
Add pad start zero to the hours and minutes values
bogiii 12d9f72
Add TimeInput story
bogiii d1c5ec7
Fix two way binding edge cases and optimize onChange triggers
bogiii 0c3383c
Remove unnecesarry Fieldset wrapper and label
bogiii 71174f0
Add TimeInput change args type
bogiii 9d7105d
Integrate TimeInput into TimePicker component
bogiii 4d5d4cc
Fix edge case of handling day period
bogiii d531e6e
Get proper hours format from the time picker component
bogiii 87bf436
Add TimeInput unit tests
bogiii 430d4ce
Update default story to reflect the component defaults
bogiii b7e2f38
Simplify passing callback function
bogiii 9b34762
Test: update element selectors
bogiii 4b6b386
Add todo comment
bogiii 5cae065
Null-ing storybook value props
bogiii 6a2cb4b
Replace minutesStep with minutesProps prop
bogiii 6250a92
Update time-input component entry props
bogiii b4c26b9
Don't trigger onChange event if the entry value is updated
bogiii 58b60be
Simplify minutesProps passing
mirka fbf1458
Simplify controlled/uncontrolled logic
mirka 2d18880
Set to WIP status
mirka 9443a7d
Merge branch 'trunk' into add/time-input
mirka f7effc9
Add changelog
mirka 5dfd31b
Update test description
bogiii File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
33 changes: 33 additions & 0 deletions
33
packages/components/src/date-time/stories/time-input.story.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| /** | ||
| * External dependencies | ||
| */ | ||
| import type { Meta, StoryFn } from '@storybook/react'; | ||
| import { action } from '@storybook/addon-actions'; | ||
|
|
||
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import { TimeInput } from '../time-input'; | ||
|
|
||
| const meta: Meta< typeof TimeInput > = { | ||
| title: 'Components/TimeInput', | ||
| component: TimeInput, | ||
| argTypes: { | ||
| onChange: { action: 'onChange', control: { type: null } }, | ||
| }, | ||
| tags: [ 'status-wip' ], | ||
| parameters: { | ||
| controls: { expanded: true }, | ||
| docs: { canvas: { sourceState: 'shown' } }, | ||
| }, | ||
| args: { | ||
| onChange: action( 'onChange' ), | ||
| }, | ||
| }; | ||
| export default meta; | ||
|
|
||
| const Template: StoryFn< typeof TimeInput > = ( args ) => { | ||
| return <TimeInput { ...args } />; | ||
| }; | ||
|
|
||
| export const Default: StoryFn< typeof TimeInput > = Template.bind( {} ); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,174 @@ | ||
| /** | ||
| * External dependencies | ||
| */ | ||
| import clsx from 'clsx'; | ||
|
|
||
| /** | ||
| * WordPress dependencies | ||
| */ | ||
| import { __ } from '@wordpress/i18n'; | ||
|
|
||
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import { | ||
| TimeWrapper, | ||
| TimeSeparator, | ||
| HoursInput, | ||
| MinutesInput, | ||
| } from '../time/styles'; | ||
| import { HStack } from '../../h-stack'; | ||
| import Button from '../../button'; | ||
| import ButtonGroup from '../../button-group'; | ||
| import { | ||
| from12hTo24h, | ||
| from24hTo12h, | ||
| buildPadInputStateReducer, | ||
| validateInputElementTarget, | ||
| } from '../utils'; | ||
| import type { TimeInputProps } from '../types'; | ||
| import type { InputChangeCallback } from '../../input-control/types'; | ||
| import { useControlledValue } from '../../utils'; | ||
|
|
||
| export function TimeInput( { | ||
| value: valueProp, | ||
| defaultValue, | ||
| is12Hour, | ||
| minutesProps, | ||
| onChange, | ||
| }: TimeInputProps ) { | ||
| const [ | ||
| value = { | ||
| hours: new Date().getHours(), | ||
| minutes: new Date().getMinutes(), | ||
| }, | ||
| setValue, | ||
| ] = useControlledValue( { | ||
| value: valueProp, | ||
| onChange, | ||
| defaultValue, | ||
| } ); | ||
| const dayPeriod = parseDayPeriod( value.hours ); | ||
| const hours12Format = from24hTo12h( value.hours ); | ||
|
|
||
| const buildNumberControlChangeCallback = ( | ||
| method: 'hours' | 'minutes' | ||
| ): InputChangeCallback => { | ||
| return ( _value, { event } ) => { | ||
| if ( ! validateInputElementTarget( event ) ) { | ||
| return; | ||
| } | ||
|
|
||
| // We can safely assume value is a number if target is valid. | ||
| const numberValue = Number( _value ); | ||
|
|
||
| setValue( { | ||
| ...value, | ||
| [ method ]: | ||
| method === 'hours' && is12Hour | ||
| ? from12hTo24h( numberValue, dayPeriod === 'PM' ) | ||
| : numberValue, | ||
| } ); | ||
| }; | ||
| }; | ||
|
|
||
| const buildAmPmChangeCallback = ( _value: 'AM' | 'PM' ) => { | ||
| return () => { | ||
| if ( dayPeriod === _value ) { | ||
| return; | ||
| } | ||
|
|
||
| setValue( { | ||
| ...value, | ||
| hours: from12hTo24h( hours12Format, _value === 'PM' ), | ||
| } ); | ||
| }; | ||
| }; | ||
|
|
||
| function parseDayPeriod( _hours: number ) { | ||
| return _hours < 12 ? 'AM' : 'PM'; | ||
| } | ||
|
|
||
| return ( | ||
| <HStack alignment="left"> | ||
| <TimeWrapper | ||
| className="components-datetime__time-field components-datetime__time-field-time" // Unused, for backwards compatibility. | ||
| > | ||
| <HoursInput | ||
| className="components-datetime__time-field-hours-input" // Unused, for backwards compatibility. | ||
| label={ __( 'Hours' ) } | ||
| hideLabelFromVision | ||
| __next40pxDefaultSize | ||
| value={ String( | ||
| is12Hour ? hours12Format : value.hours | ||
| ).padStart( 2, '0' ) } | ||
| step={ 1 } | ||
| min={ is12Hour ? 1 : 0 } | ||
| max={ is12Hour ? 12 : 23 } | ||
| required | ||
| spinControls="none" | ||
| isPressEnterToChange | ||
| isDragEnabled={ false } | ||
| isShiftStepEnabled={ false } | ||
| onChange={ buildNumberControlChangeCallback( 'hours' ) } | ||
| __unstableStateReducer={ buildPadInputStateReducer( 2 ) } | ||
| /> | ||
| <TimeSeparator | ||
| className="components-datetime__time-separator" // Unused, for backwards compatibility. | ||
| aria-hidden="true" | ||
| > | ||
| : | ||
| </TimeSeparator> | ||
| <MinutesInput | ||
| className={ clsx( | ||
| 'components-datetime__time-field-minutes-input', // Unused, for backwards compatibility. | ||
| minutesProps?.className | ||
| ) } | ||
| label={ __( 'Minutes' ) } | ||
| hideLabelFromVision | ||
| __next40pxDefaultSize | ||
| value={ String( value.minutes ).padStart( 2, '0' ) } | ||
| step={ 1 } | ||
| min={ 0 } | ||
| max={ 59 } | ||
| required | ||
| spinControls="none" | ||
| isPressEnterToChange | ||
| isDragEnabled={ false } | ||
| isShiftStepEnabled={ false } | ||
| onChange={ ( ...args ) => { | ||
| buildNumberControlChangeCallback( 'minutes' )( | ||
| ...args | ||
| ); | ||
| minutesProps?.onChange?.( ...args ); | ||
| } } | ||
| __unstableStateReducer={ buildPadInputStateReducer( 2 ) } | ||
| { ...minutesProps } | ||
| /> | ||
| </TimeWrapper> | ||
| { is12Hour && ( | ||
| <ButtonGroup | ||
| className="components-datetime__time-field components-datetime__time-field-am-pm" // Unused, for backwards compatibility. | ||
| > | ||
| <Button | ||
| className="components-datetime__time-am-button" // Unused, for backwards compatibility. | ||
| variant={ dayPeriod === 'AM' ? 'primary' : 'secondary' } | ||
| __next40pxDefaultSize | ||
| onClick={ buildAmPmChangeCallback( 'AM' ) } | ||
| > | ||
| { __( 'AM' ) } | ||
| </Button> | ||
| <Button | ||
| className="components-datetime__time-pm-button" // Unused, for backwards compatibility. | ||
| variant={ dayPeriod === 'PM' ? 'primary' : 'secondary' } | ||
| __next40pxDefaultSize | ||
| onClick={ buildAmPmChangeCallback( 'PM' ) } | ||
| > | ||
| { __( 'PM' ) } | ||
| </Button> | ||
| </ButtonGroup> | ||
| ) } | ||
| </HStack> | ||
| ); | ||
| } | ||
| export default TimeInput; |
171 changes: 171 additions & 0 deletions
171
packages/components/src/date-time/time-input/test/index.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,171 @@ | ||
| /** | ||
| * External dependencies | ||
| */ | ||
| import { render, screen } from '@testing-library/react'; | ||
| import userEvent from '@testing-library/user-event'; | ||
|
|
||
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import TimeInput from '..'; | ||
|
|
||
| describe( 'TimeInput', () => { | ||
| it( 'should call onChange with updated values | 24-hours format', async () => { | ||
| const user = userEvent.setup(); | ||
|
|
||
| const timeInputValue = { hours: 0, minutes: 0 }; | ||
| const onChangeSpy = jest.fn(); | ||
|
|
||
| render( | ||
| <TimeInput | ||
| defaultValue={ timeInputValue } | ||
| onChange={ onChangeSpy } | ||
| /> | ||
| ); | ||
|
|
||
| const hoursInput = screen.getByRole( 'spinbutton', { name: 'Hours' } ); | ||
| const minutesInput = screen.getByRole( 'spinbutton', { | ||
| name: 'Minutes', | ||
| } ); | ||
|
|
||
| await user.clear( minutesInput ); | ||
| await user.type( minutesInput, '35' ); | ||
| await user.keyboard( '{Tab}' ); | ||
|
|
||
| expect( onChangeSpy ).toHaveBeenCalledWith( { hours: 0, minutes: 35 } ); | ||
| onChangeSpy.mockClear(); | ||
|
|
||
| await user.clear( hoursInput ); | ||
| await user.type( hoursInput, '12' ); | ||
| await user.keyboard( '{Tab}' ); | ||
|
|
||
| expect( onChangeSpy ).toHaveBeenCalledWith( { | ||
| hours: 12, | ||
| minutes: 35, | ||
| } ); | ||
| onChangeSpy.mockClear(); | ||
|
|
||
| await user.clear( hoursInput ); | ||
| await user.type( hoursInput, '23' ); | ||
| await user.keyboard( '{Tab}' ); | ||
|
|
||
| expect( onChangeSpy ).toHaveBeenCalledWith( { | ||
| hours: 23, | ||
| minutes: 35, | ||
| } ); | ||
| onChangeSpy.mockClear(); | ||
|
|
||
| await user.clear( minutesInput ); | ||
| await user.type( minutesInput, '0' ); | ||
| await user.keyboard( '{Tab}' ); | ||
|
|
||
| expect( onChangeSpy ).toHaveBeenCalledWith( { hours: 23, minutes: 0 } ); | ||
| } ); | ||
|
|
||
| it( 'should call onChange with updated values | 12-hours format', async () => { | ||
| const user = userEvent.setup(); | ||
|
|
||
| const timeInputValue = { hours: 0, minutes: 0 }; | ||
| const onChangeSpy = jest.fn(); | ||
|
|
||
| render( | ||
| <TimeInput | ||
| is12Hour | ||
| defaultValue={ timeInputValue } | ||
| onChange={ onChangeSpy } | ||
| /> | ||
| ); | ||
|
|
||
| const hoursInput = screen.getByRole( 'spinbutton', { name: 'Hours' } ); | ||
| const minutesInput = screen.getByRole( 'spinbutton', { | ||
| name: 'Minutes', | ||
| } ); | ||
| const amButton = screen.getByRole( 'button', { name: 'AM' } ); | ||
| const pmButton = screen.getByRole( 'button', { name: 'PM' } ); | ||
|
|
||
| // TODO: Update assert these states through the accessibility tree rather than through styles, see: https://github.com/WordPress/gutenberg/issues/61163 | ||
| expect( amButton ).toHaveClass( 'is-primary' ); | ||
bogiii marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| expect( pmButton ).not.toHaveClass( 'is-primary' ); | ||
| expect( hoursInput ).not.toHaveValue( 0 ); | ||
| expect( hoursInput ).toHaveValue( 12 ); | ||
|
|
||
| await user.clear( minutesInput ); | ||
| await user.type( minutesInput, '35' ); | ||
| await user.keyboard( '{Tab}' ); | ||
|
|
||
| expect( onChangeSpy ).toHaveBeenCalledWith( { hours: 0, minutes: 35 } ); | ||
| expect( amButton ).toHaveClass( 'is-primary' ); | ||
|
|
||
| await user.clear( hoursInput ); | ||
| await user.type( hoursInput, '12' ); | ||
| await user.keyboard( '{Tab}' ); | ||
|
|
||
| expect( onChangeSpy ).toHaveBeenCalledWith( { hours: 0, minutes: 35 } ); | ||
|
|
||
| await user.click( pmButton ); | ||
| expect( onChangeSpy ).toHaveBeenCalledWith( { | ||
| hours: 12, | ||
| minutes: 35, | ||
| } ); | ||
| expect( pmButton ).toHaveClass( 'is-primary' ); | ||
| } ); | ||
|
|
||
| it( 'should call onChange with defined minutes steps', async () => { | ||
| const user = userEvent.setup(); | ||
|
|
||
| const timeInputValue = { hours: 0, minutes: 0 }; | ||
| const onChangeSpy = jest.fn(); | ||
|
|
||
| render( | ||
| <TimeInput | ||
| defaultValue={ timeInputValue } | ||
| minutesProps={ { step: 5 } } | ||
| onChange={ onChangeSpy } | ||
| /> | ||
| ); | ||
|
|
||
| const minutesInput = screen.getByRole( 'spinbutton', { | ||
| name: 'Minutes', | ||
| } ); | ||
|
|
||
| await user.clear( minutesInput ); | ||
| await user.keyboard( '{ArrowUp}' ); | ||
|
|
||
| expect( minutesInput ).toHaveValue( 5 ); | ||
|
|
||
| await user.keyboard( '{ArrowUp}' ); | ||
| await user.keyboard( '{ArrowUp}' ); | ||
|
|
||
| expect( minutesInput ).toHaveValue( 15 ); | ||
|
|
||
| await user.keyboard( '{ArrowDown}' ); | ||
|
|
||
| expect( minutesInput ).toHaveValue( 10 ); | ||
|
|
||
| await user.clear( minutesInput ); | ||
| await user.type( minutesInput, '44' ); | ||
| await user.keyboard( '{Tab}' ); | ||
|
|
||
| expect( minutesInput ).toHaveValue( 45 ); | ||
|
|
||
| await user.clear( minutesInput ); | ||
| await user.type( minutesInput, '51' ); | ||
| await user.keyboard( '{Tab}' ); | ||
|
|
||
| expect( minutesInput ).toHaveValue( 50 ); | ||
| } ); | ||
|
|
||
| it( 'should reflect changes to the value prop', () => { | ||
| const { rerender } = render( | ||
| <TimeInput value={ { hours: 0, minutes: 0 } } /> | ||
| ); | ||
| rerender( <TimeInput value={ { hours: 1, minutes: 2 } } /> ); | ||
|
|
||
| expect( | ||
| screen.getByRole( 'spinbutton', { name: 'Hours' } ) | ||
| ).toHaveValue( 1 ); | ||
| expect( | ||
| screen.getByRole( 'spinbutton', { name: 'Minutes' } ) | ||
| ).toHaveValue( 2 ); | ||
| } ); | ||
| } ); | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.