From 23b5b8f890d301902232a3750eef0a422ea64dfe Mon Sep 17 00:00:00 2001 From: Kai Hao Date: Fri, 14 Aug 2020 16:51:12 +0800 Subject: [PATCH] Refactor to function component (#24348) * Refactor tests to use RTL * Add test for ordering of the month/day input * Add storybook for * Refactor * Coding style updates as per suggested in code reviews --- .../components/src/date-time/stories/time.js | 26 + .../date-time/test/__snapshots__/time.js.snap | 689 ------------------ .../components/src/date-time/test/time.js | 290 +++++--- packages/components/src/date-time/time.js | 534 ++++++-------- 4 files changed, 423 insertions(+), 1116 deletions(-) create mode 100644 packages/components/src/date-time/stories/time.js delete mode 100644 packages/components/src/date-time/test/__snapshots__/time.js.snap diff --git a/packages/components/src/date-time/stories/time.js b/packages/components/src/date-time/stories/time.js new file mode 100644 index 0000000000000..52ed6fb7ce832 --- /dev/null +++ b/packages/components/src/date-time/stories/time.js @@ -0,0 +1,26 @@ +/** + * Internal dependencies + */ +import TimePicker from '../time'; + +/** + * External dependencies + */ +import { date, boolean } from '@storybook/addon-knobs'; +import { noop } from 'lodash'; + +export default { title: 'Components/TimePicker', component: TimePicker }; + +export const _default = () => { + return ( + + ); +}; diff --git a/packages/components/src/date-time/test/__snapshots__/time.js.snap b/packages/components/src/date-time/test/__snapshots__/time.js.snap deleted file mode 100644 index ae17692f74a5f..0000000000000 --- a/packages/components/src/date-time/test/__snapshots__/time.js.snap +++ /dev/null @@ -1,689 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`TimePicker matches the snapshot when the is12hour prop is false 1`] = ` -
-
- - Date - -
-
- -
-
- -
-
- -
-
-
-
- - Time - -
-
- - - -
- -
-
-
-`; - -exports[`TimePicker matches the snapshot when the is12hour prop is specified 1`] = ` -
-
- - Date - -
-
- -
-
- -
-
- -
-
-
-
- - Time - -
-
- - - -
- - - AM - - - PM - - - -
-
-
-`; - -exports[`TimePicker matches the snapshot when the is12hour prop is true 1`] = ` -
-
- - Date - -
-
- -
-
- -
-
- -
-
-
-
- - Time - -
-
- - - -
- - - AM - - - PM - - - -
-
-
-`; - -exports[`TimePicker matches the snapshot when the is12hour prop is undefined 1`] = ` -
-
- - Date - -
-
- -
-
- -
-
- -
-
-
-
- - Time - -
-
- - - -
- -
-
-
-`; diff --git a/packages/components/src/date-time/test/time.js b/packages/components/src/date-time/test/time.js index c55ef19127509..32a47ca9ed2dc 100644 --- a/packages/components/src/date-time/test/time.js +++ b/packages/components/src/date-time/test/time.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { shallow } from 'enzyme'; +import { render, fireEvent, screen } from '@testing-library/react'; /** * Internal dependencies @@ -9,202 +9,170 @@ import { shallow } from 'enzyme'; import TimePicker from '../time'; describe( 'TimePicker', () => { - it( 'matches the snapshot when the is12hour prop is true', () => { - const wrapper = shallow( - - ); - expect( wrapper ).toMatchSnapshot(); - } ); - - it( 'matches the snapshot when the is12hour prop is false', () => { - const wrapper = shallow( - - ); - expect( wrapper ).toMatchSnapshot(); - } ); - - it( 'matches the snapshot when the is12hour prop is specified', () => { - const wrapper = shallow( - - ); - expect( wrapper ).toMatchSnapshot(); - } ); - - it( 'matches the snapshot when the is12hour prop is undefined', () => { - const wrapper = shallow( - - ); - expect( wrapper ).toMatchSnapshot(); - } ); - - it( 'should call onChange with an updated day', () => { + it( 'should call onChange with updated date values', () => { const onChangeSpy = jest.fn(); - const picker = shallow( - - ); - const input = picker - .find( '.components-datetime__time-field-day-input' ) - .at( 0 ); - input.simulate( 'change', { target: { value: '10' } } ); - input.simulate( 'blur' ); - expect( onChangeSpy ).toHaveBeenCalledWith( '1986-10-10T11:00:00' ); - } ); - it( 'should call onChange with an updated month', () => { - const onChangeSpy = jest.fn(); - const picker = shallow( + render( ); - const input = picker - .find( '.components-datetime__time-field-month-select' ) - .at( 0 ); - input.simulate( 'change', { target: { value: '12' } } ); - input.simulate( 'blur' ); + + const monthInput = screen.getByLabelText( 'Month' ); + const dayInput = screen.getByLabelText( 'Day' ); + const yearInput = screen.getByLabelText( 'Year' ); + const hoursInput = screen.getByLabelText( 'Hours' ); + const minutesInput = screen.getByLabelText( 'Minutes' ); + + fireEvent.change( monthInput, { target: { value: '12' } } ); + fireEvent.blur( monthInput ); + expect( onChangeSpy ).toHaveBeenCalledWith( '1986-12-18T11:00:00' ); - } ); + onChangeSpy.mockClear(); - it( 'should call onChange with an updated year', () => { - const onChangeSpy = jest.fn(); - const picker = shallow( - - ); - const input = picker - .find( '.components-datetime__time-field-year-input' ) - .at( 0 ); - input.simulate( 'change', { target: { value: '2018' } } ); - input.simulate( 'blur' ); - expect( onChangeSpy ).toHaveBeenCalledWith( '2018-10-18T11:00:00' ); + fireEvent.change( dayInput, { target: { value: '22' } } ); + fireEvent.blur( dayInput ); + + expect( onChangeSpy ).toHaveBeenCalledWith( '1986-12-22T11:00:00' ); + onChangeSpy.mockClear(); + + fireEvent.change( yearInput, { target: { value: '2018' } } ); + fireEvent.blur( yearInput ); + + expect( onChangeSpy ).toHaveBeenCalledWith( '2018-12-22T11:00:00' ); + onChangeSpy.mockClear(); + + fireEvent.change( hoursInput, { target: { value: '12' } } ); + fireEvent.blur( hoursInput ); + + expect( onChangeSpy ).toHaveBeenCalledWith( '2018-12-22T12:00:00' ); + onChangeSpy.mockClear(); + + fireEvent.change( minutesInput, { target: { value: '35' } } ); + fireEvent.blur( minutesInput ); + + expect( onChangeSpy ).toHaveBeenCalledWith( '2018-12-22T12:35:00' ); + onChangeSpy.mockClear(); } ); it( 'should call onChange with an updated hour (12-hour clock)', () => { const onChangeSpy = jest.fn(); - const picker = shallow( + + render( ); - const input = picker - .find( '.components-datetime__time-field-hours-input' ) - .at( 0 ); - input.simulate( 'change', { target: { value: '10' } } ); - input.simulate( 'blur' ); + + const hoursInput = screen.getByLabelText( 'Hours' ); + + fireEvent.change( hoursInput, { target: { value: '10' } } ); + fireEvent.blur( hoursInput ); + expect( onChangeSpy ).toHaveBeenCalledWith( '1986-10-18T10:00:00' ); } ); it( 'should not call onChange with an updated hour (12-hour clock) if the hour is out of bounds', () => { const onChangeSpy = jest.fn(); - const picker = shallow( + + render( ); - const input = picker - .find( '.components-datetime__time-field-hours-input' ) - .at( 0 ); - input.simulate( 'change', { target: { value: '22' } } ); - input.simulate( 'blur' ); + + const hoursInput = screen.getByLabelText( 'Hours' ); + + fireEvent.change( hoursInput, { target: { value: '22' } } ); + fireEvent.blur( hoursInput ); + expect( onChangeSpy ).not.toHaveBeenCalled(); } ); it( 'should call onChange with an updated hour (24-hour clock)', () => { const onChangeSpy = jest.fn(); - const picker = shallow( + + render( ); - const input = picker - .find( '.components-datetime__time-field-hours-input' ) - .at( 0 ); - input.simulate( 'change', { target: { value: '22' } } ); - input.simulate( 'blur' ); - expect( onChangeSpy ).toHaveBeenCalledWith( '1986-10-18T22:00:00' ); - } ); - it( 'should call onChange with an updated minute', () => { - const onChangeSpy = jest.fn(); - const picker = shallow( - - ); - const input = picker - .find( '.components-datetime__time-field-minutes-input' ) - .at( 0 ); - input.simulate( 'change', { target: { value: '10' } } ); - input.simulate( 'blur' ); - expect( onChangeSpy ).toHaveBeenCalledWith( '1986-10-18T11:10:00' ); + const hoursInput = screen.getByLabelText( 'Hours' ); + + fireEvent.change( hoursInput, { target: { value: '22' } } ); + fireEvent.blur( hoursInput ); + + expect( onChangeSpy ).toHaveBeenCalledWith( '1986-10-18T22:00:00' ); } ); it( 'should not call onChange with an updated minute if out of bounds', () => { const onChangeSpy = jest.fn(); - const picker = shallow( + + render( ); - const input = picker - .find( '.components-datetime__time-field-minutes-input' ) - .at( 0 ); - input.simulate( 'change', { target: { value: '99' } } ); - input.simulate( 'blur' ); + + const minutesInput = screen.getByLabelText( 'Minutes' ); + + fireEvent.change( minutesInput, { target: { value: '99' } } ); + fireEvent.blur( minutesInput ); + expect( onChangeSpy ).not.toHaveBeenCalled(); } ); it( 'should switch to PM correctly', () => { const onChangeSpy = jest.fn(); - const button = shallow( + + render( ); - const pmButton = button.find( '.components-datetime__time-pm-button' ); - pmButton.simulate( 'click' ); + + const pmButton = screen.getByText( 'PM' ); + + fireEvent.click( pmButton ); + expect( onChangeSpy ).toHaveBeenCalledWith( '1986-10-18T23:00:00' ); } ); it( 'should switch to AM correctly', () => { const onChangeSpy = jest.fn(); - const button = shallow( + + render( ); - const pmButton = button.find( '.components-datetime__time-am-button' ); - pmButton.simulate( 'click' ); + + const amButton = screen.getByText( 'AM' ); + + fireEvent.click( amButton ); + expect( onChangeSpy ).toHaveBeenCalledWith( '1986-10-18T11:00:00' ); } ); it( 'should truncate at the minutes on change', () => { const onChangeSpy = jest.fn(); - const wrapper = shallow( + render( { /> ); - const minuteInput = wrapper.find( 'input[aria-label="Minutes"]' ); + const minutesInput = screen.getByLabelText( 'Minutes' ); - minuteInput.simulate( 'change', { target: { value: '22' } } ); - minuteInput.simulate( 'blur' ); + fireEvent.change( minutesInput, { target: { value: '22' } } ); + fireEvent.blur( minutesInput ); expect( onChangeSpy ).toHaveBeenCalledWith( '1986-10-18T23:22:00' ); } ); + + it( 'should reset the date when currentTime changed', () => { + const onChangeSpy = jest.fn(); + + const { rerender } = render( + + ); + + rerender( + + ); + + expect( screen.getByLabelText( 'Month' ).value ).toBe( '07' ); + expect( screen.getByLabelText( 'Day' ).value ).toBe( '13' ); + expect( screen.getByLabelText( 'Year' ).value ).toBe( '2020' ); + expect( screen.getByLabelText( 'Hours' ).value ).toBe( '06' ); + expect( screen.getByLabelText( 'Minutes' ).value ).toBe( '00' ); + /** + * This is not ideal, but best of we can do for now until we refactor + * AM/PM into accessible elements, like radio buttons. + */ + expect( + screen.getByText( 'AM' ).classList.contains( 'is-primary' ) + ).toBe( false ); + expect( + screen.getByText( 'PM' ).classList.contains( 'is-primary' ) + ).toBe( true ); + } ); + + it( 'should have different layouts/orders for 12/24 hour formats', () => { + const onChangeSpy = jest.fn(); + + const { rerender } = render( +
+ + + ); + + const form = screen.getByRole( 'form' ); + + let monthInputIndex = [].indexOf.call( + form.elements, + screen.getByLabelText( 'Month' ) + ); + let dayInputIndex = [].indexOf.call( + form.elements, + screen.getByLabelText( 'Day' ) + ); + + expect( monthInputIndex < dayInputIndex ).toBe( true ); + + rerender( +
+ + + ); + + monthInputIndex = [].indexOf.call( + form.elements, + screen.getByLabelText( 'Month' ) + ); + dayInputIndex = [].indexOf.call( + form.elements, + screen.getByLabelText( 'Day' ) + ); + + expect( monthInputIndex > dayInputIndex ).toBe( true ); + } ); } ); diff --git a/packages/components/src/date-time/time.js b/packages/components/src/date-time/time.js index 1b502d70aaf66..5df865e95e27d 100644 --- a/packages/components/src/date-time/time.js +++ b/packages/components/src/date-time/time.js @@ -8,7 +8,12 @@ import moment from 'moment'; /** * WordPress dependencies */ -import { Component } from '@wordpress/element'; +import { + createElement, + useState, + useMemo, + useEffect, +} from '@wordpress/element'; import { __ } from '@wordpress/i18n'; /** @@ -23,347 +28,260 @@ import TimeZone from './timezone'; */ const TIMEZONELESS_FORMAT = 'YYYY-MM-DDTHH:mm:ss'; -class TimePicker extends Component { - constructor() { - super( ...arguments ); - this.state = { - day: '', - month: '', - year: '', - hours: '', - minutes: '', - am: '', - date: null, - }; - this.changeDate = this.changeDate.bind( this ); - this.updateMonth = this.updateMonth.bind( this ); - this.onChangeMonth = this.onChangeMonth.bind( this ); - this.updateDay = this.updateDay.bind( this ); - this.onChangeDay = this.onChangeDay.bind( this ); - this.updateYear = this.updateYear.bind( this ); - this.onChangeYear = this.onChangeYear.bind( this ); - this.updateHours = this.updateHours.bind( this ); - this.updateMinutes = this.updateMinutes.bind( this ); - this.onChangeHours = this.onChangeHours.bind( this ); - this.onChangeMinutes = this.onChangeMinutes.bind( this ); - this.renderMonth = this.renderMonth.bind( this ); - this.renderDay = this.renderDay.bind( this ); - this.renderDayMonthFormat = this.renderDayMonthFormat.bind( this ); - } +/** + * + * A shared component to parse, validate, and handle remounting of the underlying form field element like and - - - - - - - - - - - - - - - ); - } + + + + + + + + + + + + + + + ); - renderDay( day ) { - return ( -
- -
- ); - } - - renderDayMonthFormat( is12Hour ) { - const { day, month } = this.state; - const layout = [ this.renderDay( day ), this.renderMonth( month ) ]; - return is12Hour ? layout : layout.reverse(); - } + const dayMonthFormat = is12Hour ? ( + <> + { dayFormat } + { monthFormat } + + ) : ( + <> + { monthFormat } + { dayFormat } + + ); - render() { - const { is12Hour } = this.props; - const { year, minutes, hours, am } = this.state; + return ( +
+
+ + { __( 'Date' ) } + +
+ { dayMonthFormat } - return ( -
-
- - { __( 'Date' ) } - -
- { this.renderDayMonthFormat( is12Hour ) } -
- -
+
+
-
+
+
-
- - { __( 'Time' ) } - -
-
- -
-
-
- ); - } + + + + + ); } export default TimePicker;