Skip to content

Commit

Permalink
DateTimePicker: set PM hours correctly (#36878)
Browse files Browse the repository at this point in the history
* DateTimePicker: Extract `from12hTo24h` function

* DateTimePicker: correctly convert hours from 12 to 24 format when updating the time

* CHANGELOG

* Only apply the value adjustment when the hours input updates

* Add unit test to check setting hours with PM period selected

* Update unit test on the assumption that 12am should be intended as midnight
  • Loading branch information
ciampo authored and noisysocks committed Nov 29, 2021
1 parent a72da92 commit 3a9eb13
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 8 deletions.
4 changes: 4 additions & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@

- Added a `showTooltip` prop to `ToggleGroupControlOption` in order to display tooltip text (using `<Tooltip />`). ([#36726](https://github.com/WordPress/gutenberg/pull/36726)).

### Bug Fix

- Fixed a bug which prevented setting `PM` hours correctly in the `DateTimePicker` ([#36878](https://github.com/WordPress/gutenberg/pull/36878)).

## 19.0.2 (2021-11-15)

- Remove erroneous use of `??=` syntax from `build-module`.
Expand Down
34 changes: 32 additions & 2 deletions packages/components/src/date-time/test/time.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,13 @@ describe( 'TimePicker', () => {
fireEvent.change( hoursInput, { target: { value: '12' } } );
fireEvent.blur( hoursInput );

expect( onChangeSpy ).toHaveBeenCalledWith( '2018-12-22T12:00:00' );
expect( onChangeSpy ).toHaveBeenCalledWith( '2018-12-22T00:00:00' );
onChangeSpy.mockClear();

fireEvent.change( minutesInput, { target: { value: '35' } } );
fireEvent.blur( minutesInput );

expect( onChangeSpy ).toHaveBeenCalledWith( '2018-12-22T12:35:00' );
expect( onChangeSpy ).toHaveBeenCalledWith( '2018-12-22T00:35:00' );
onChangeSpy.mockClear();
} );

Expand Down Expand Up @@ -169,6 +169,36 @@ describe( 'TimePicker', () => {
expect( onChangeSpy ).toHaveBeenCalledWith( '1986-10-18T11:00:00' );
} );

it( 'should allow to set the time correctly when the PM period is selected', () => {
const onChangeSpy = jest.fn();

render(
<TimePicker
currentTime="1986-10-18T11:00:00"
onChange={ onChangeSpy }
is12Hour
/>
);

const pmButton = screen.getByText( 'PM' );
fireEvent.click( pmButton );

const hoursInput = screen.getByLabelText( 'Hours' );
fireEvent.change( hoursInput, { target: { value: '6' } } );
fireEvent.blur( hoursInput );

// When clicking on 'PM', we expect the time to be 11pm
expect( onChangeSpy ).toHaveBeenNthCalledWith(
1,
'1986-10-18T23:00:00'
);
// When changing the hours to '6', we expect the time to be 6pm
expect( onChangeSpy ).toHaveBeenNthCalledWith(
2,
'1986-10-18T18:00:00'
);
} );

it( 'should truncate at the minutes on change', () => {
const onChangeSpy = jest.fn();

Expand Down
20 changes: 14 additions & 6 deletions packages/components/src/date-time/time.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ import TimeZone from './timezone';
*/
const TIMEZONELESS_FORMAT = 'YYYY-MM-DDTHH:mm:ss';

function from12hTo24h( hours, isPm ) {
return isPm ? ( ( hours % 12 ) + 12 ) % 24 : hours % 12;
}

/**
* <UpdateOnBlurAsIntegerField>
* A shared component to parse, validate, and handle remounting of the underlying form field element like <input> and <select>.
Expand Down Expand Up @@ -117,8 +121,16 @@ export function TimePicker( { is12Hour, currentTime, onChange } ) {
}

function update( name, value ) {
// If the 12-hour format is being used and the 'PM' period is selected, then
// the incoming value (which ranges 1-12) should be increased by 12 to match
// the expected 24-hour format.
let adjustedValue = value;
if ( name === 'hours' && is12Hour ) {
adjustedValue = from12hTo24h( value, am === 'PM' );
}

// Clone the date and call the specific setter function according to `name`.
const newDate = date.clone()[ name ]( value );
const newDate = date.clone()[ name ]( adjustedValue );
changeDate( newDate );
}

Expand All @@ -132,11 +144,7 @@ export function TimePicker( { is12Hour, currentTime, onChange } ) {

const newDate = date
.clone()
.hours(
value === 'PM'
? ( ( parsedHours % 12 ) + 12 ) % 24
: parsedHours % 12
);
.hours( from12hTo24h( parsedHours, value === 'PM' ) );

changeDate( newDate );
};
Expand Down

0 comments on commit 3a9eb13

Please sign in to comment.