Size Change: +102 kB (8%) 🔍

Total Size: 1.27 MB

_{compressed-size-action}

@retrofox I have a huge queue of tasks to go through 🙂 Will try go get to this as soon as possible.

In the meantime, can you explain please what is the value added for users here? At first glance, this change seems prioritizing developers convenience over users experience.

It has changed considerably, and now it has nice support for a11y. Not sure if it will enough.

Also, if possible, can you provide a list of the a11y improvements landed in react-datepicker? That would greatly help.

@retrofox I have a huge queue of tasks to go through 🙂 Will try go get to this as soon as possible.

I bet you have. Thanks in advance.

In the meantime, can you explain please what is the value added for users here? At first glance, this change seems prioritizing developers convenience over users experience.

Not at all :-). From my POV, the goal for these improvements is to be able to:

• highlight site events in the calendar. Very useful when users want to organize their site agenda. For instance, when scheduling a post you will get aware that maybe you're going to push two posts on the same date (we need to work on the a11y for this feature)
• Show information about events, by date. It's tackled using a Tooltip component. Again, we need to work on the a11y here too.

We tried to do the same with the current dev libraries, I ran into many issues, here yes, from the development point of view.

It has changed considerably, and now it has nice support for a11y. Not sure if it will enough.

Also, if possible, can you provide a list of the a11y improvements landed in react-datepicker? That would greatly help.

These issues reported by you here:

• not operable with a keyboard
• empty and not focusable prev/next month links
• day names not in relationship with the day numbers
• arguable values for the day aria-labels, e.g. day-15
• missing labels for the hour/minutes fields

Not 100% sure whether the last two ones are rightly handled, but if not, I'm guessing tackle them it isn't a big deal. The most important issues were about keyboard focus issues.

Thanks for your feedback, Andrea.

I'm definitely excited for this — the DateTime component has been rather difficult to work with as a third party. Hope to see this land and bring some needed improvements to the date picker :) Thanks @retrofox!

@tellthemachines if I remember correctly you worked a lot on the current implementation. Any thoughts here? 🙂

Any chance that this switch will solve also #15348 ?
As mentioned on the issue, it depends also on the Date/Time setting set in WordPress but in some cases all the days in the month are announced in a way that screen readers really can't make use of. For example:

• Monday, j F 2020
• Tuesday, j F 2020
• Wednesday, j F 2020
• etc.

This has always been broken since WP 5.0 and still needs to be fixed...

Any chance that this switch will solve also #15348 ?
As mentioned on the issue, it depends also on the Date/Time setting set in WordPress but in some cases all the days in the month are announced in a way that screen readers really can't make use of. For example:

• Monday, j F 2020
• Tuesday, j F 2020
• Wednesday, j F 2020
• etc.

This has always been broken since WP 5.0 and still needs to be fixed...

Happy to at least try to bring a solution.

Any chance that this switch will solve also #15348 ?

Let me cross the comment link and update the PR description, but in short, the answer is yes. It's easily doable.

Tested a bit this PR and, from an accessibility perspective, there are a few problems to solve mainly related to keyboard navigation with screen readers and labelling. On a general level, I'm not sure react-datepicker has full support for internationalization and RTL languages; this is a technical part that should be evaluated separately.

It's going to be a bit long so I'm going to use one comment for each issue 🙂

First: keyboard navigation with screen readers

VoiceOver on mac works pretty differently from screen readers on Windows. The Windows ones (e.g NVDA, JAWS) use two main modes to navigate the content. Let's call these modes "browse mode" and "focus mode", even if the terminology changes depending on the screen reader. In browse mode, screen readers intercept most of the key presses for their internal usage. For example, the arrow keys are used to navigate the content. In this mode, key strokes are not passed to the browser so any script relying on keyboard events simply doesn't work. This is the main navigation mode used by users. Instead, focus mode is mainly used for form elements ot with some specific ARIA roles.

Why this matters:

Since selecting the day in the calendar month relies on keyboard events, in browse mode the day selection totally fails. See the animated GIF below that illustrates the behavior with Firefox and NVDA:

• Left and right arrows navigate the content by character: this is the expected behavior with a screen reader but in the calendar this is supposed to select the previous or next day.
• Up and down arrows randomly make the day selection jump by 2 or 3 days: with a screen reader, these keys are expected to move to the previous / next "line" but in the calendar they're supposed to move to the previous / next week.

The reason why the calendar works in the current react-dates based implementation is that the DayPicker component uses an ARIA role="application". Instead, react-datepicker doesn't. This role, amongst other things, instructs screen readers to stop intercepting key presses so they're passed to the browser and any script relying on keyboard events "magically" works again.

The role="application" is very debated amongst specialists as it brings in also undesired side effects and its usage should be limited to very rare cases but in this specific case I can't think of another way to "fix" the day selection. Without this, the calendar would be totally unusable to Windows screen reader users.

Possible options:

1
Ideally, role="application" should be added to the container of the days:

I'm not sure there's a way to do that, though.

2
Alternatively, I see react-datepicker has a container prop that can be used to add a wrapper around the whole widget. This could be used to wrap everything in a div with the role and an aria-label. More or less:

const RoleApplicationContainer = ( { className, children } ) => {
	return (
	  <div role="application" aria-label={ __( 'Calendar' ) } className={ className }>
	      { children }
	  </div>
	);
};

and then pass the custom container to the date picker: calendarContainer={ RoleApplicationContainer }. However, this would wrap the whole widget within the ARIA role, which isn't ideal.

3
Maybe better: report upstream and ask to add the role + aria-label and wait for the change to be released.

For the records, I tested the option 2) and it fixes the problem.

Tested a bit this PR and, from an accessibility perspective, there are a few problems ...

Thanks for your awesome feedback, Andrea. Very useful <3. I'm going to create a checklist in the PR description to organize the next actions to take. Please, feel free to add/edit.

Labelling and localization / internationalization

While the dates are "automatically" translated based on the locale, I see that react-datepicker uses some strings for its on user interface. I'm not sure these strings are translatable: they appear to be hardcoded. Also, at least one of these strings seems to be a bit pointless. For example:

• The month container has an aria-label="month 2020-09" which doesn't appear to be translatable and I'm not sure it adds great value: it's on an unfocusable div element with no ARIA role. Also, the month is already visible in plain text.
• More importantly, all the day buttons have an aria label that by default has also a text prefix "Choose":

aria-label="Choose Friday, October 2nd, 2020"

The "Choose" prefix is hardcoded and seems it's not translatable: https://github.com/Hacker0x01/react-datepicker/blob/6d789febf11630ef4080084ad4744add92a89ab8/src/day.jsx#L240-L253

This prefix can be changed via the ariaLabelPrefixWhenEnabled prop and I'd recommend to remove it by passing an empty string. It's redundant for screen reader users and a problem for speech recognition software users. However, we'd need to investigate on the aria-label in the first place because we need a way to communicate the Events, more on this later.

Other strings that seem not translatable:

• https://github.com/Hacker0x01/react-datepicker/blob/6d789febf11630ef4080084ad4744add92a89ab8/src/calendar.jsx#L59-L74
• https://github.com/Hacker0x01/react-datepicker/blob/6d789febf11630ef4080084ad4744add92a89ab8/src/calendar.jsx#L457-L460

I do see these can be changed via props but I'm not sure I understand how can they be translated?

One more interesting question is: does react-datepicker support RTL languages. In the react-dates codebase I can see methods to support RTL but at a first glance I can't find anything related to RTL in react-datepicker.

Labelling and Events

The main purpose of this change is to add "events" on the calendar days. To start with, these events would be other posts published or scheduled for that day. Screenshot:

However, the tooltips alone are only a "visual thing". The tooltip documentation itself specifies that:

Accessibility note: the tooltip text is hidden from screen readers and assistive technologies that understand ARIA. To make it accessible, use an aria-label attribute on the same element the tooltip is applied to, preferably using the same text used for the tooltip.

It is also worth reminding that the tooltip is rendered in the DOM very far from the element it is applied to so it can't be reached (nor it should be) via keyboard navigation. Also, tooltips can't contain interactive elements (e.g. links) because they would not be available to keyboard users. A tooltip is only a visual "hint" that should be accompanied with other accessible text to convey the same information to assistive technology users.

Basically, when navigating through the days with a screen reader, there's no information whatsoever about the Events on that day that can be announced. So the point is: how do we communicate the Events information in a way that is available on the day buttons themselves?

As noted in a previous comment, the buttons use an aria-label that overrides their content:

aria-label="Choose Friday, October 2nd, 2020"

As far as I see, there's no way to change this aria-label. Only the "Choose" prefix can be changed.

There are a few options that could be used to add the Events info within the day buttons but they would need the ability to change or remove the aria-label. This would require some investigation:

• Is there a way to change or, better, remove the aria-label?
• Can this be reported upstream to ask for a prop to remove the aria-label?
• If not, can we "hack" it in some way?

If the aria-label could be removed, we could just use renderDayContents() to add some visually hidden text with all the information we want to provide about the Events.

This is probably the most challenging point for the Events implementation.

Other issues worth mentioning (some of them are present also on the current implementation):

• the prev/next month buttons should have an aria-label and tooltip: this can be fixed by passing a label prop with appropriate text to the buttons (the label prop will also add the tooltip)
• when pressing the prev/next month buttons there's no audible confirmation message for screen reader users: they won't have a clue what happened
• the prev/next month buttons contrast ratio is too low
• not sure why the prev/next month buttons use dashicons, i thought SVG icons were the standard in Gutenberg
• the Events tooltips are displayed only on hover: they should be displayed also on focus
• the Events are rendered in an unordered list: this is okay but a bit pointless because the list is visually reset via CSS and the tooltip can't be announced by assistive technologies so the semantics used isn't greatly relevant
• the day buttons contrast ratio on hover is not sufficient: color: #757575, background: #f0f0f0, contrast ratio 4.04:1
• the selected day button has no semantic indication it's the selected one
• when setting a date by clicking (or pressing Enter/Spacebar) on a day, there's no audible confirmation message
• Calendar Help
  • it will need to be updated with the new keyboard shortcuts (also, the headings hierarchy is not correct)
  • it would be better to make the Calendar Help a modal dialog and also add a X close button at the top to improve the tab order
  • the keyboard shortcuts should use the same pattern that is used for the editor keyboard shortcuts, and we should make sure everything is announced correctly by screen readers
  • the usage of the <abbr> element is not correct
• clicking "Reset" empties all the fields but the month one and the Reset button disappears: not sure if the fields should be emptied in the first place: best would be to "reset" to previous date or to now
• after clicking "Reset", clicking or navigating via keyboard through the input fields triggers a JS error and the editor crashes this appears to happen a bit inconsistently: click randomly on the input fields and outside them multiple times to trigger the error Uncaught RangeError: Invalid time value

FYI: The increase in build size that the bot is reporting worried me at first, but I think this is just due to the fact that components has grown in size due to other changes since this PR was created. Rebasing on the main branch should once again show the bundle size going down as a result of this PR.

FYI: The increase in build size that the bot is reporting worried me at first, but I think this is just due to the fact that components has grown in size due to other changes since this PR was created. Rebasing on the main branch should once again show the bundle size going down as a result of this PR.

do you mean to the following?

do you mean to the following?

Yes, that's what I meant. The 101KB gzipped would have been a worryingly large increase for changes to a single component, on the order of 2.5 times the size of react + react-dom, or 1.5 times the size of the full moment with all locales.

do you mean to the following?

Yes, that's what I meant. The 101KB gzipped would have been a worryingly large increase for changes to a single component, on the order of 2.5 times the size of react + react-dom, or 1.5 times the size of the full moment with all locales.

Thanks. Nice, we're going to create a new PR only for the component replacement. There, we're going pay attention to this. Do you mind if I add you as a reviewer?

Do you mind if I add you as a reviewer?

Not at all, happy to take a look! 🙂

The issue #15348 came up in a Core Editor triage held by @gwwar Kerry and myself. It looks like this PR has stopped up.
@retrofox Damian. It would be great with an update here. How can we help move this PR forward? What is holding it back?

The issue #15348 came up in a Core Editor triage held by @gwwar Kerry and myself.

It's nice to hear that :-)

It looks like this PR has stopped up.
@retrofox Damian. It would be great with an update here. How can we help move this PR forward? What is holding it back?

That would be very nice. However, this PR suggests a huge refactoring that will take a lot of time, time that now I'm not sure whether I'll be able to provide.
We have a related PR, smaller, ready to review. Would you mind take a look at this one before introducing us to a bigger change?

Thanks, folks.

With #43005 merged, I believe that this PR can probably be closed.

Of course, more a11y feedback on the current status of the component is welcome!

cc @noisysocks