fix(#3118): DatePicker calendar cannot be opened by keyboard (#3145)

* fix(#3118): DatePicker calendar cannot be opened by keyboard

1. Include DatePicker/DateRangePicker in tab order.
2. Fix focus management so that ArrowLeft/ArrowRight moves focus to and from the Calendar button.
3. Fix focus ring so that the focus state on the button is rendered independent of the focus state for the DatePicker input group.

* fix(#3118): update tests

* fix(#3118): DatePicker Enter on last segment should not focus Calendar Button

TODO: With DateRangePicker, Enter should still advance from last segment of start date to first segment of end date.

* Fix date range picker navigation

* Fix

* fix(#3118): Calendar add stopPropagation on keyboard navigation

Per #3145 (comment)

Co-authored-by: Devon Govett <devongovett@gmail.com>
Co-authored-by: Danni <drobinson@livefyre.com>

Author: 3 people

packages/@react-aria/calendar/src/useCalendarGrid.ts

@@ -63,22 +63,27 @@ export function useCalendarGrid(props: AriaCalendarGridProps, state: CalendarSta
         break;
       case 'PageUp':
         e.preventDefault();
+        e.stopPropagation();
         state.focusPreviousSection(e.shiftKey);
         break;
       case 'PageDown':
         e.preventDefault();
+        e.stopPropagation();
         state.focusNextSection(e.shiftKey);
         break;
       case 'End':
         e.preventDefault();
+        e.stopPropagation();
         state.focusSectionEnd();
         break;
       case 'Home':
         e.preventDefault();
+        e.stopPropagation();
         state.focusSectionStart();
         break;
       case 'ArrowLeft':
         e.preventDefault();
+        e.stopPropagation();
         if (direction === 'rtl') {
           state.focusNextDay();
         } else {
@@ -87,10 +92,12 @@ export function useCalendarGrid(props: AriaCalendarGridProps, state: CalendarSta
         break;
       case 'ArrowUp':
         e.preventDefault();
+        e.stopPropagation();
         state.focusPreviousRow();
         break;
       case 'ArrowRight':
         e.preventDefault();
+        e.stopPropagation();
         if (direction === 'rtl') {
           state.focusPreviousDay();
         } else {
@@ -99,6 +106,7 @@ export function useCalendarGrid(props: AriaCalendarGridProps, state: CalendarSta
         break;
       case 'ArrowDown':
         e.preventDefault();
+        e.stopPropagation();
         state.focusNextRow();
         break;
       case 'Escape':

packages/@react-aria/datepicker/src/useDateField.ts

@@ -62,8 +62,6 @@ export function useDateField<T extends DateValue>(props: AriaDateFieldProps<T>,
     labelElementType: 'span'
   });
 
-  let groupProps = useDatePickerGroup(state, ref);
-
   let {focusWithinProps} = useFocusWithin({
     onBlurWithin() {
       state.confirmPlaceholder();
@@ -83,6 +81,7 @@ export function useDateField<T extends DateValue>(props: AriaDateFieldProps<T>,
     : [descProps['aria-describedby'], fieldProps['aria-describedby']].filter(Boolean).join(' ') || undefined;
   let propsFocusManager = props[focusManagerSymbol];
   let focusManager = useMemo(() => propsFocusManager || createFocusManager(ref), [propsFocusManager, ref]);
+  let groupProps = useDatePickerGroup(state, ref, props[roleSymbol] === 'presentation');
 
   // Pass labels and other information to segments.
   hookData.set(state, {

packages/@react-aria/datepicker/src/useDatePicker.ts

@@ -17,7 +17,7 @@ import {CalendarProps} from '@react-types/calendar';
 import {createFocusManager} from '@react-aria/focus';
 import {DatePickerState} from '@react-stately/datepicker';
 import {filterDOMProps, mergeProps, useDescription, useId} from '@react-aria/utils';
-import {HTMLAttributes, RefObject} from 'react';
+import {HTMLAttributes, RefObject, useMemo} from 'react';
 // @ts-ignore
 import intlMessages from '../intl/*.json';
 import {roleSymbol} from './useDateField';
@@ -68,6 +68,7 @@ export function useDatePicker<T extends DateValue>(props: AriaDatePickerProps<T>
   let descProps = useDescription(description);
   let ariaDescribedBy = [descProps['aria-describedby'], fieldProps['aria-describedby']].filter(Boolean).join(' ') || undefined;
   let domProps = filterDOMProps(props);
+  let focusManager = useMemo(() => createFocusManager(ref), [ref]);
 
   return {
     groupProps: mergeProps(domProps, groupProps, fieldProps, descProps, {
@@ -79,7 +80,6 @@ export function useDatePicker<T extends DateValue>(props: AriaDatePickerProps<T>
     labelProps: {
       ...labelProps,
       onClick: () => {
-        let focusManager = createFocusManager(ref);
         focusManager.focusFirst();
       }
     },
@@ -106,7 +106,6 @@ export function useDatePicker<T extends DateValue>(props: AriaDatePickerProps<T>
     buttonProps: {
       ...descProps,
       id: buttonId,
-      excludeFromTabOrder: true,
       'aria-haspopup': 'dialog',
       'aria-label': formatMessage('calendar'),
       'aria-labelledby': `${labelledBy} ${buttonId}`,

packages/@react-aria/datepicker/src/useDatePickerGroup.ts

@@ -1,18 +1,47 @@
+import {createFocusManager, getFocusableTreeWalker} from '@react-aria/focus';
 import {DateFieldState, DatePickerState, DateRangePickerState} from '@react-stately/datepicker';
-import {getFocusableTreeWalker} from '@react-aria/focus';
 import {KeyboardEvent} from '@react-types/shared';
 import {mergeProps} from '@react-aria/utils';
-import {RefObject} from 'react';
+import {RefObject, useMemo} from 'react';
+import {useLocale} from '@react-aria/i18n';
 import {usePress} from '@react-aria/interactions';
 
-export function useDatePickerGroup(state: DatePickerState | DateRangePickerState | DateFieldState, ref: RefObject<HTMLElement>) {
+export function useDatePickerGroup(state: DatePickerState | DateRangePickerState | DateFieldState, ref: RefObject<HTMLElement>, disableArrowNavigation?: boolean) {
+  let {direction} = useLocale();
+  let focusManager = useMemo(() => createFocusManager(ref), [ref]);
+
   // Open the popover on alt + arrow down
   let onKeyDown = (e: KeyboardEvent) => {
     if (e.altKey && (e.key === 'ArrowDown' || e.key === 'ArrowUp') && 'setOpen' in state) {
       e.preventDefault();
       e.stopPropagation();
       state.setOpen(true);
     }
+
+    if (disableArrowNavigation) {
+      return;
+    }
+
+    switch (e.key) {
+      case 'ArrowLeft':
+        e.preventDefault();
+        e.stopPropagation();
+        if (direction === 'rtl') {
+          focusManager.focusNext();
+        } else {
+          focusManager.focusPrevious();
+        }
+        break;
+      case 'ArrowRight':
+        e.preventDefault();
+        e.stopPropagation();
+        if (direction === 'rtl') {
+          focusManager.focusPrevious();
+        } else {
+          focusManager.focusNext();
+        }
+        break;
+    }
   };
 
   // Focus the first placeholder segment from the end on mouse down/touch up in the field.

packages/@react-aria/datepicker/src/useDateRangePicker.ts

@@ -87,7 +87,11 @@ export function useDateRangePicker<T extends DateValue>(props: AriaDateRangePick
   });
 
   let ariaDescribedBy = [descProps['aria-describedby'], fieldProps['aria-describedby']].filter(Boolean).join(' ') || undefined;
-  let focusManager = useMemo(() => createFocusManager(ref), [ref]);
+  let focusManager = useMemo(() => createFocusManager(ref, {
+    // Exclude the button from the focus manager.
+    accept: element => element.id !== buttonId
+  }), [ref, buttonId]);
+
   let commonFieldProps = {
     [focusManagerSymbol]: focusManager,
     [roleSymbol]: 'presentation',
@@ -121,7 +125,6 @@ export function useDateRangePicker<T extends DateValue>(props: AriaDateRangePick
     buttonProps: {
       ...descProps,
       id: buttonId,
-      excludeFromTabOrder: true,
       'aria-haspopup': 'dialog',
       'aria-label': formatMessage('calendar'),
       'aria-labelledby': `${labelledBy} ${buttonId}`,

packages/@react-aria/datepicker/src/useDateSegment.ts

@@ -31,7 +31,7 @@ export interface DateSegmentAria {
  */
 export function useDateSegment(segment: DateSegment, state: DateFieldState, ref: RefObject<HTMLElement>): DateSegmentAria {
   let enteredKeys = useRef('');
-  let {locale, direction} = useLocale();
+  let {locale} = useLocale();
   let displayNames = useDisplayNames();
   let {ariaLabel, ariaLabelledBy, ariaDescribedBy, focusManager} = hookData.get(state);
 
@@ -114,31 +114,13 @@ export function useDateSegment(segment: DateSegment, state: DateFieldState, ref:
     }
 
     switch (e.key) {
-      case 'ArrowLeft':
-        e.preventDefault();
-        e.stopPropagation();
-        if (direction === 'rtl') {
-          focusManager.focusNext({tabbable: true});
-        } else {
-          focusManager.focusPrevious({tabbable: true});
-        }
-        break;
-      case 'ArrowRight':
-        e.preventDefault();
-        e.stopPropagation();
-        if (direction === 'rtl') {
-          focusManager.focusPrevious({tabbable: true});
-        } else {
-          focusManager.focusNext({tabbable: true});
-        }
-        break;
       case 'Enter':
         e.preventDefault();
         e.stopPropagation();
         if (segment.isPlaceholder && !state.isReadOnly) {
           state.confirmPlaceholder(segment.type);
         }
-        focusManager.focusNext({tabbable: true});
+        focusManager.focusNext();
         break;
       case 'Tab':
         break;
@@ -184,7 +166,7 @@ export function useDateSegment(segment: DateSegment, state: DateFieldState, ref:
         } else {
           break;
         }
-        focusManager.focusNext({tabbable: true});
+        focusManager.focusNext();
         break;
       case 'day':
       case 'hour':
@@ -233,7 +215,7 @@ export function useDateSegment(segment: DateSegment, state: DateFieldState, ref:
         if (Number(numberValue + '0') > segment.maxValue || newValue.length >= String(segment.maxValue).length) {
           enteredKeys.current = '';
           if (shouldSetValue) {
-            focusManager.focusNext({tabbable: true});
+            focusManager.focusNext();
           }
         } else {
           enteredKeys.current = newValue;

packages/@react-aria/focus/src/FocusScope.tsx

@@ -44,7 +44,9 @@ interface FocusManagerOptions {
   /** Whether to only include tabbable elements, or all focusable elements. */
   tabbable?: boolean,
   /** Whether focus should wrap around when it reaches the end of the scope. */
-  wrap?: boolean
+  wrap?: boolean,
+  /** A callback that determines whether the given element is focused. */
+  accept?: (node: Element) => boolean
 }
 
 export interface FocusManager {
@@ -487,7 +489,9 @@ export function getFocusableTreeWalker(root: HTMLElement, opts?: FocusManagerOpt
 
         if ((node as HTMLElement).matches(selector)
           && isElementVisible(node as HTMLElement)
-          && (!scope || isElementInScope(node as HTMLElement, scope))) {
+          && (!scope || isElementInScope(node as HTMLElement, scope))
+          && (!opts?.accept || opts.accept(node as Element))
+        ) {
           return NodeFilter.FILTER_ACCEPT;
         }
 
@@ -506,13 +510,13 @@ export function getFocusableTreeWalker(root: HTMLElement, opts?: FocusManagerOpt
 /**
  * Creates a FocusManager object that can be used to move focus within an element.
  */
-export function createFocusManager(ref: RefObject<HTMLElement>): FocusManager {
+export function createFocusManager(ref: RefObject<HTMLElement>, defaultOptions: FocusManagerOptions = {}): FocusManager {
   return {
     focusNext(opts: FocusManagerOptions = {}) {
       let root = ref.current;
-      let {from, tabbable, wrap} = opts;
+      let {from, tabbable = defaultOptions.tabbable, wrap = defaultOptions.wrap, accept = defaultOptions.accept} = opts;
       let node = from || document.activeElement;
-      let walker = getFocusableTreeWalker(root, {tabbable});
+      let walker = getFocusableTreeWalker(root, {tabbable, accept});
       if (root.contains(node)) {
         walker.currentNode = node;
       }
@@ -526,11 +530,11 @@ export function createFocusManager(ref: RefObject<HTMLElement>): FocusManager {
       }
       return nextNode;
     },
-    focusPrevious(opts: FocusManagerOptions = {}) {
+    focusPrevious(opts: FocusManagerOptions = defaultOptions) {
       let root = ref.current;
-      let {from, tabbable, wrap} = opts;
+      let {from, tabbable = defaultOptions.tabbable, wrap = defaultOptions.wrap, accept = defaultOptions.accept} = opts;
       let node = from || document.activeElement;
-      let walker = getFocusableTreeWalker(root, {tabbable});
+      let walker = getFocusableTreeWalker(root, {tabbable, accept});
       if (root.contains(node)) {
         walker.currentNode = node;
       } else {
@@ -550,20 +554,20 @@ export function createFocusManager(ref: RefObject<HTMLElement>): FocusManager {
       }
       return previousNode;
     },
-    focusFirst(opts = {}) {
+    focusFirst(opts = defaultOptions) {
       let root = ref.current;
-      let {tabbable} = opts;
-      let walker = getFocusableTreeWalker(root, {tabbable});
+      let {tabbable = defaultOptions.tabbable, accept = defaultOptions.accept} = opts;
+      let walker = getFocusableTreeWalker(root, {tabbable, accept});
       let nextNode = walker.nextNode() as HTMLElement;
       if (nextNode) {
         focusElement(nextNode, true);
       }
       return nextNode;
     },
-    focusLast(opts = {}) {
+    focusLast(opts = defaultOptions) {
       let root = ref.current;
-      let {tabbable} = opts;
-      let walker = getFocusableTreeWalker(root, {tabbable});
+      let {tabbable = defaultOptions.tabbable, accept = defaultOptions.accept} = opts;
+      let walker = getFocusableTreeWalker(root, {tabbable, accept});
       let next = last(walker);
       if (next) {
         focusElement(next, true);

packages/@react-spectrum/datepicker/src/DatePicker.tsx

@@ -62,6 +62,12 @@ function DatePicker<T extends DateValue>(props: SpectrumDatePickerProps<T>, ref:
     autoFocus
   });
 
+  let {isFocused: isFocusedButton, focusProps: focusPropsButton} = useFocusRing({
+    within: false,
+    isTextInput: false,
+    autoFocus
+  });
+
   let className = classNames(
     styles,
     'spectrum-InputGroup',
@@ -71,7 +77,7 @@ function DatePicker<T extends DateValue>(props: SpectrumDatePickerProps<T>, ref:
       'is-disabled': isDisabled,
       'is-hovered': isHovered,
       'is-focused': isFocused,
-      'focus-ring': isFocusVisible
+      'focus-ring': isFocusVisible && !isFocusedButton
     }
   );
 
@@ -136,7 +142,7 @@ function DatePicker<T extends DateValue>(props: SpectrumDatePickerProps<T>, ref:
           onOpenChange={setOpen}
           shouldFlip={props.shouldFlip}>
           <FieldButton
-            {...buttonProps}
+            {...mergeProps(buttonProps, focusPropsButton)}
             UNSAFE_className={classNames(styles, 'spectrum-FieldButton')}
             isQuiet={isQuiet}
             validationState={state.validationState}

packages/@react-spectrum/datepicker/src/DateRangePicker.tsx

@@ -62,6 +62,12 @@ function DateRangePicker<T extends DateValue>(props: SpectrumDateRangePickerProp
     autoFocus
   });
 
+  let {isFocused: isFocusedButton, focusProps: focusPropsButton} = useFocusRing({
+    within: false,
+    isTextInput: false,
+    autoFocus
+  });
+
   let className = classNames(
     styles,
     'spectrum-InputGroup',
@@ -71,7 +77,7 @@ function DateRangePicker<T extends DateValue>(props: SpectrumDateRangePickerProp
       'is-disabled': isDisabled,
       'is-hovered': isHovered,
       'is-focused': isFocused,
-      'focus-ring': isFocusVisible
+      'focus-ring': isFocusVisible && !isFocusedButton
     }
   );
 
@@ -150,7 +156,7 @@ function DateRangePicker<T extends DateValue>(props: SpectrumDateRangePickerProp
           onOpenChange={setOpen}
           shouldFlip={props.shouldFlip}>
           <FieldButton
-            {...buttonProps}
+            {...mergeProps(buttonProps, focusPropsButton)}
             UNSAFE_className={classNames(styles, 'spectrum-FieldButton')}
             isQuiet={isQuiet}
             validationState={state.validationState}