Datepicker: Add isInvalidDay support by psealock · Pull Request #12962 · WordPress/gutenberg

psealock · 2018-12-18T00:23:58Z

Description

Add the concept of unselectable days to the DatePicker's calendar by exposing react-dates prop isOutsideRange. Here is an example with Mondays made as invalid selections:

What about invalidating dates via the inputs?

The changes here affect only the calendar (<DatePicker /> component) and not the inputs (<TimePicker />). Ideally, the same validation is passed along to the inputs to prevent selection of invalid days via the keyboard. That will take some design work to surface why an invalidation has occurred. I propose to handle that separately.

Testing instructions

Since the concept of invalid days has no meaning for the text editor, changing code temporarily is the only way to evaluate this change.

Add this prop to invalidate Mondays

isInvalidDate={ ( date ) => 1 === date.getDay() }

to the implementation of <DatePicker />,

gutenberg/packages/components/src/date-time/index.js

Lines 48 to 51 in 0c734e1

    
           <DatePicker 
        
           	currentDate={ currentDate } 
        
           	onChange={ onChange } 
        
           />

Go to a new post /wp-admin/post-new.php#/.
Open the date picker.
See Mondays are not selectable via the calendar.
Using VoiceOver, confirm focus of an invalid calendar date adds a "Not available" to the description.

Types of changes

New feature: Expose a prop, isInvalidDay , to <DatePicker />.

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.

timmyc

Looking good to me, just one minor comment.

timmyc · 2019-01-22T21:00:01Z

packages/components/src/date-time/date.js

Probably not totally required but should we provide a default prop for this of false just to be extra explicit?

Thanks @timmyc !

I could set a default prop or just { isInvalidDay = false } = this.props, but I think its best to pass along an undefined value to react-dates's own default prop for isOutsideRange:

https://github.com/airbnb/react-dates/blob/d572bea9029739089bd200026be3aad7684a8b02/src/components/DayPickerSingleDateController.jsx#L100

The reason is because React handles an undefined prop differently than null or possibly false (https://reactjs.org/docs/react-component.html#defaultprops).

timmyc

This is looking good to me.

aduth

The DateTime component has a README.md . The prop should be documented.

psealock · 2019-01-23T20:03:12Z

Thanks for the review @aduth. Changes made to DateTime Readme.

aduth · 2019-01-24T02:04:28Z

Moment is not otherwise exposed as part of the public interface, and I don't think we'd want to start now (related: #11418).

What would you think about passing the Date object instead?

https://momentjs.com/docs/#/displaying/as-javascript-date/

psealock · 2019-01-24T19:12:59Z

@aduth I've modified how isInvalidDay is passed on to react-dates such that the prop function's argument is a Date object instead of Moment.

I updated the test instructions on this PR. Thanks again for the input.

packages/components/src/date-time/README.md

aduth · 2019-01-24T20:05:17Z

packages/components/src/date-time/date.js

Sorry, I'm nitpicking at this point, but the term "Day" is ambiguous to me, where the value itself to check is a Date object. I'm also a bit wary about the negation here, regardless of what react-dates is doing, since it seems a bit unnecessary to reverse what is essentially a validity check.

Thoughts on isValidDate instead?

There's a further argument about the naming of the prop implying that its value is itself a boolean, when in fact it's a callback function. I'm not sure what precedent there is here for these sorts of callback functions. I'm not overly concerned about it.

I'm also a bit wary about the negation here, regardless of what react-dates is doing, since it seems a bit unnecessary to reverse what is essentially a validity check.

One advantage of the negation relates to the optionality of the prop. By default, all dates are valid. It would seem odd to me that by adding a positively-framed check, isValidDate, I'd have to explicitly return true to enable a date that is otherwise enabled by default.

the term "Day" is ambiguous to me

Great point.

My slight preference is isInvalidDate, but I can go with the positive isValidDate as well.

I'd be fine with isInvalidDate

Cool, done and rebased.

aduth

Looks great 👍 Thanks for your patience.

…rnmobile/372-enter-key-detection-to-title * 'master' of https://github.com/WordPress/gutenberg: (29 commits) Update for RangeControl documentation (#12564) Plugin: Deprecate gutenberg_load_list_reusable_blocks (#13456) Update the columns attribute in onSelectImages so that if images are removed via the media modal, the columns can't be higher than the new number of images (#13488) Replace the fullscreen "exit" icon with a back arrow (#13403) Include :visited links in button color (#12183) Amazon Kindle block (#13510) Plugin: Deprecate gutenberg_prepare_blocks_for_js (#13457) Add watcher on Linux: change fs to node-watch (#13448) Plugin: Deprecate `gutenberg` theme support (#13458) Datepicker: Add inValidDay support (#12962) Block Switcher: Render disabled button even if multi-selection (#13431) Plugin: Deprecate gutenberg_register_post_types (#13468) Plugin: Deprecate register_tinymce_scripts (#13466) Set minimum of words for RSS excerpt (#13502) Plugin: Deprecate gutenberg_get_block_categories (#13454) Plugin: Deprecate gutenberg_content_block_version (#13469) API Fetch: Expose nonce on created middleware function (#13451) Plugin: Remove list screens integrations (#13459) Plugin: Remove core-defined block detection functions (#13467) Spec Parser: Move generated spec parser to package (#13493) ...

This was referenced Dec 20, 2018

Add Single Date Picker woocommerce/woocommerce-admin#1069

Merged

Single Date Picker: A11y review woocommerce/woocommerce-admin#1145

Closed

psealock mentioned this pull request Jan 1, 2019

Advanced Filters: Add DatePicker woocommerce/woocommerce-admin#1146

Merged

designsimply added [Priority] High Used to indicate top priority items that need quick attention [Package] Date /packages/date labels Jan 17, 2019

designsimply added this to the 4.9 (Gutenberg) milestone Jan 17, 2019

designsimply requested a review from a team January 17, 2019 21:42

psealock force-pushed the add/datepicker-isOutsideRange-support branch from 4269e8f to 912ca49 Compare January 20, 2019 19:53

youknowriad modified the milestones: 4.9 (Gutenberg), 5.0 (Gutenberg) Jan 21, 2019

timmyc reviewed Jan 22, 2019

View reviewed changes

psealock force-pushed the add/datepicker-isOutsideRange-support branch from 912ca49 to caa76ba Compare January 22, 2019 21:15

timmyc approved these changes Jan 23, 2019

View reviewed changes

aduth requested changes Jan 23, 2019

View reviewed changes

psealock force-pushed the add/datepicker-isOutsideRange-support branch 2 times, most recently from 757ec8d to 33d6217 Compare January 23, 2019 19:36

aduth reviewed Jan 24, 2019

View reviewed changes

packages/components/src/date-time/README.md Outdated Show resolved Hide resolved

aduth reviewed Jan 24, 2019

View reviewed changes

Datepicker: Add inValidDay support

006fdfc

psealock force-pushed the add/datepicker-isOutsideRange-support branch from 53e3aef to 006fdfc Compare January 24, 2019 22:38

aduth approved these changes Jan 25, 2019

View reviewed changes

aduth merged commit f7ade90 into WordPress:master Jan 25, 2019

psealock deleted the add/datepicker-isOutsideRange-support branch January 27, 2019 20:03

This was referenced Feb 4, 2019

Bump plugin version to 5.0.0-rc.1 #13652

Merged

Bump plugin version to 5.0.0 #13689

Closed

Bump plugin version to 5.0.0 #13692

Merged

youknowriad pushed a commit that referenced this pull request Mar 6, 2019

Datepicker: Add inValidDay support (#12962)

34842b4

youknowriad pushed a commit that referenced this pull request Mar 6, 2019

Datepicker: Add inValidDay support (#12962)

0ab9c05

psealock mentioned this pull request Mar 7, 2019

Calendar: Use isInvalidDate to disallow date selections woocommerce/woocommerce-admin#1685

Merged

	<DatePicker
	currentDate={ currentDate }
	onChange={ onChange }
	/>

Conversation

psealock commented Dec 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

What about invalidating dates via the inputs?

Testing instructions

Types of changes

Checklist:

Uh oh!

timmyc left a comment

Choose a reason for hiding this comment

Uh oh!

timmyc Jan 22, 2019

Choose a reason for hiding this comment

Uh oh!

psealock Jan 22, 2019

Choose a reason for hiding this comment

Uh oh!

timmyc left a comment

Choose a reason for hiding this comment

Uh oh!

aduth left a comment

Choose a reason for hiding this comment

Uh oh!

psealock commented Jan 23, 2019

Uh oh!

aduth commented Jan 24, 2019

Uh oh!

psealock commented Jan 24, 2019

Uh oh!

Uh oh!

aduth Jan 24, 2019

Choose a reason for hiding this comment

Uh oh!

psealock Jan 24, 2019

Choose a reason for hiding this comment

Uh oh!

aduth Jan 24, 2019

Choose a reason for hiding this comment

Uh oh!

psealock Jan 24, 2019

Choose a reason for hiding this comment

Uh oh!

aduth left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

psealock commented Dec 18, 2018 •

edited

Loading