Fixing date picker and field components event props not working #3605

ktabors · 2022-10-05T23:36:17Z

The key and focus events will only happen when navigating the field and picker button. They (key events) won't occur in the calendar picker. And focus isn't lost when moving to the picker button or when the picker is open. Blur happens when you leave the component.

Changed the state API by adding isFocused and setFocused. This is replicating how picker managed these events.

✅ Pull Request Checklist:

Included link to corresponding React Spectrum GitHub Issue.
Added/updated unit tests and storybook for this change (for new code or code which already has tests).
Filled out test instructions.
Updated documentation (if it already exists for this component).
Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

Added a story called "all events" to TimeField, DateField, DatePicker, and DateRangePicker. It allows us to test and confirm all the missing events.

🧢 Your Project:

RSP

adobe-bot · 2022-10-06T00:26:25Z

Build successful! 🎉

adobe-bot · 2022-10-11T05:08:57Z

Build successful! 🎉

adobe-bot · 2022-10-11T16:19:44Z

Build successful! 🎉

ktabors · 2022-10-11T16:34:28Z

packages/@react-spectrum/datepicker/stories/DateField.stories.tsx

@@ -124,6 +124,10 @@ storiesOf('Date and Time/DateField', module)
  .add(
    'showFormatHelpText',
    () => render({showFormatHelpText: true})
+  )
+  .add(
+    'all the events',


better story name?

ktabors · 2022-10-11T16:34:33Z

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

+        }
+      },
+      onFocus(e: ReactFocusEvent) {
+        if (state.isFocused) {


This was modeled on how Picker handles these events. Picker had the state.isFocused and e.currentTarget.contains(e.relatedTarget as Node) logic. In this case, the state.isFocused is to not keep calling focus when moving between the segments and the trigger button. The e.currentTarget.contains(e.relatedTarget as Node) keeps the blur from being called when moving between segments and to the trigger button. The state.isOpen keeps the focus, blur, and keyup/down events from being called while using the calendar popover. There weren't comments on this in Picker so I'm not sure if we want them here.

While this logic is identical for the two picker hooks, date and time fields don't have a popover and state.isOpen, so it didn't seem worthwhile to generalize.

snowystinger

Can we have tests for these changes?

adobe-bot · 2022-10-13T21:54:23Z

Build successful! 🎉

packages/@react-spectrum/datepicker/test/DateField.test.js

packages/@react-spectrum/datepicker/test/DatePicker.test.js

adobe-bot · 2022-10-17T23:21:28Z

Build successful! 🎉

adobe-bot · 2022-10-17T23:21:29Z

API Changes

@react-stately/datepicker

DateFieldState

 DateFieldState {
   calendar: Calendar
   clearSegment: (SegmentType) => void
   confirmPlaceholder: () => void
   dateFormatter: DateFormatter
   dateValue: Date
   decrement: (SegmentType) => void
   decrementPage: (SegmentType) => void
   formatValue: (FieldOptions) => string
   granularity: Granularity
   increment: (SegmentType) => void
   incrementPage: (SegmentType) => void
   isDisabled: boolean
+  isFocused: boolean
   isReadOnly: boolean
   isRequired: boolean
   maxGranularity: 'year' | 'month' | Granularity
   segments: Array<DateSegment>
+  setFocused: (boolean) => void
   setSegment: (SegmentType, number) => void
   setValue: (DateValue) => void
   validationState: ValidationState
   value: DateValue

it changed:

useDateSegment
useDateField
useTimeField
useDateFieldState
useTimeFieldState

DatePickerState

 DatePickerState {
   close: () => void
   dateValue: DateValue
   formatValue: (string, FieldOptions) => string
   granularity: Granularity
   hasTime: boolean
+  isFocused: boolean
   isOpen: boolean
   open: () => void
   setDateValue: (CalendarDate) => void
+  setFocused: (boolean) => void
   setOpen: (boolean) => void
   setTimeValue: (TimeValue) => void
   setValue: (DateValue) => void
   timeValue: TimeValue
   validationState: ValidationState
   value: DateValue
 }

it changed:

useDatePicker
useDatePickerState

DateRangePickerState

 DateRangePickerState {
   close: () => void
   dateRange: DateRange
   formatValue: (string, FieldOptions) => {
     start: string
   end: string
 }
   granularity: Granularity
   hasTime: boolean
+  isFocused: boolean
   isOpen: boolean
   open: () => void
   setDate: ('start' | 'end', DateValue) => void
   setDateRange: (DateRange) => void
   setDateTime: ('start' | 'end', DateValue) => void
+  setFocused: (boolean) => void
   setOpen: (boolean) => void
   setTime: ('start' | 'end', TimeValue) => void
   setTimeRange: (TimeRange) => void
   setValue: (DateRange) => void
   toggle: () => void
   validationState: ValidationState
   value: DateRange
 }

it changed:

useDateRangePicker
useDateRangePickerState

snowystinger

one last question, i don't seem to get an onKeyDown when I press enter on the button to open the calendar popover. what's swallowing that event?

otherwise works as expected in chrome and safari

ktabors · 2022-10-18T20:44:26Z

one last question, i don't seem to get an onKeyDown when I press enter on the button to open the calendar popover. what's swallowing that event?

~~Good catch, the key events should be added to buttonProps. I'll add a test for that too.~~
This appears to be a general DialogTrigger issue and not specific to DatePicker. I tested and confirmed this in the DialogTrigger stories and made the following codesandbox to demonstrate it.

https://codesandbox.io/s/shy-bush-bnlplz?file=/src/App.js The story has a div wrapping a TextField and DialogTrigger, with onKeyDown and onKeyUp event listeners attached to the div. It logs the key press events in the TextField. When focused on the ActionButton of the DialogTrigger, a enter key press only logs an onKeyUp. While still on the ActionButton of the DialogTrigger, if you press another key, like left arrow, both the onKeyUp and onKeyDown are logged.

reidbarber

LGTM

adobe-bot · 2022-10-26T16:59:43Z

Build successful! 🎉

adobe-bot · 2022-10-26T16:59:43Z

API Changes

@react-stately/datepicker

DateFieldState

 DateFieldState {
   calendar: Calendar
   clearSegment: (SegmentType) => void
   confirmPlaceholder: () => void
   dateFormatter: DateFormatter
   dateValue: Date
   decrement: (SegmentType) => void
   decrementPage: (SegmentType) => void
   formatValue: (FieldOptions) => string
   granularity: Granularity
   increment: (SegmentType) => void
   incrementPage: (SegmentType) => void
   isDisabled: boolean
+  isFocused: boolean
   isReadOnly: boolean
   isRequired: boolean
   maxGranularity: 'year' | 'month' | Granularity
   segments: Array<DateSegment>
+  setFocused: (boolean) => void
   setSegment: (SegmentType, number) => void
   setValue: (DateValue) => void
   validationState: ValidationState
   value: DateValue

it changed:

useDateSegment
useDateField
useTimeField
useDateFieldState
useTimeFieldState

DatePickerState

 DatePickerState {
   close: () => void
   dateValue: DateValue
   formatValue: (string, FieldOptions) => string
   granularity: Granularity
   hasTime: boolean
+  isFocused: boolean
   isOpen: boolean
   open: () => void
   setDateValue: (CalendarDate) => void
+  setFocused: (boolean) => void
   setOpen: (boolean) => void
   setTimeValue: (TimeValue) => void
   setValue: (DateValue) => void
   timeValue: TimeValue
   validationState: ValidationState
   value: DateValue
 }

it changed:

useDatePicker
useDatePickerState

DateRangePickerState

 DateRangePickerState {
   close: () => void
   dateRange: DateRange
   formatValue: (string, FieldOptions) => {
     start: string
   end: string
 }
   granularity: Granularity
   hasTime: boolean
+  isFocused: boolean
   isOpen: boolean
   open: () => void
   setDate: ('start' | 'end', DateValue) => void
   setDateRange: (DateRange) => void
   setDateTime: ('start' | 'end', DateValue) => void
+  setFocused: (boolean) => void
   setOpen: (boolean) => void
   setTime: ('start' | 'end', TimeValue) => void
   setTimeRange: (TimeRange) => void
   setValue: (DateRange) => void
   toggle: () => void
   validationState: ValidationState
   value: DateRange
 }

it changed:

useDateRangePicker
useDateRangePickerState

ethangclark · 2022-10-27T16:38:02Z

Can we get a release cut? The date picker API is completely busted without this.

snowystinger · 2022-10-27T16:51:17Z

It will go out in our next release which we are prepping. It takes time to get a quality release out and we appreciate your patience. In the meantime, you could probably use capture events on a parent to work around this. Also, we don't stopPropagation on the focus events I don't think, so you can just listen to those on a parent element.

devongovett · 2022-10-27T21:44:03Z

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

+        }
+
+        state.setFocused(false);
+      }


FYI, this is basically recreating the logic that useFocusWithin already implements. We could probably combine.

Fixing date picker and field components event props not working

d91a547

ktabors marked this pull request as draft October 5, 2022 23:41

removing onFocusChange

c4e866e

ktabors added 2 commits October 10, 2022 21:32

refactor to make events work more like Picker

d560ed2

fixing punctuation

7f53d3f

finishing the refactor for the other hooks and components

2fe574b

ktabors commented Oct 11, 2022

View reviewed changes

ktabors marked this pull request as ready for review October 11, 2022 16:35

snowystinger reviewed Oct 12, 2022

View reviewed changes

Merge branch 'main' into date_events

64b5c1a

ktabors added 2 commits October 13, 2022 22:35

adding focus and key event tests for date components

c32699c

forgot to remove an 'only'

4ad9697

snowystinger reviewed Oct 14, 2022

View reviewed changes

ktabors added 2 commits October 17, 2022 16:12

fixing the dialog close focus to button issue

ff83762

forgot the async and waitfor in daterangepicker

a83ca49

adobe deleted a comment from adobe-bot Oct 17, 2022

snowystinger approved these changes Oct 17, 2022

View reviewed changes

reidbarber approved these changes Oct 26, 2022

View reviewed changes

Merge branch 'main' into date_events

0c16053

ktabors merged commit 4e68e5a into main Oct 26, 2022

ktabors deleted the date_events branch October 26, 2022 17:21

ktabors added the needs testing label Oct 26, 2022

devongovett reviewed Oct 27, 2022

View reviewed changes

ktabors mentioned this pull request Oct 28, 2022

Refactoring date components to use useFocusWithin #3694

Merged

5 tasks

ktabors added release and removed needs testing labels Nov 7, 2022

dannify removed the release label Feb 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing date picker and field components event props not working #3605

Fixing date picker and field components event props not working #3605

ktabors commented Oct 5, 2022 •

edited

Loading

adobe-bot commented Oct 6, 2022

adobe-bot commented Oct 11, 2022

adobe-bot commented Oct 11, 2022

ktabors Oct 11, 2022 •

edited

Loading

ktabors Oct 11, 2022

snowystinger left a comment

adobe-bot commented Oct 13, 2022

adobe-bot commented Oct 17, 2022

adobe-bot commented Oct 17, 2022

snowystinger left a comment •

edited

Loading

ktabors commented Oct 18, 2022 •

edited

Loading

reidbarber left a comment

adobe-bot commented Oct 26, 2022

adobe-bot commented Oct 26, 2022

ethangclark commented Oct 27, 2022

snowystinger commented Oct 27, 2022

devongovett Oct 27, 2022

Fixing date picker and field components event props not working #3605

Fixing date picker and field components event props not working #3605

Conversation

ktabors commented Oct 5, 2022 • edited Loading

✅ Pull Request Checklist:

📝 Test Instructions:

🧢 Your Project:

adobe-bot commented Oct 6, 2022

adobe-bot commented Oct 11, 2022

adobe-bot commented Oct 11, 2022

ktabors Oct 11, 2022 • edited Loading

Choose a reason for hiding this comment

ktabors Oct 11, 2022

Choose a reason for hiding this comment

snowystinger left a comment

Choose a reason for hiding this comment

adobe-bot commented Oct 13, 2022

adobe-bot commented Oct 17, 2022

adobe-bot commented Oct 17, 2022

API Changes

@react-stately/datepicker

DateFieldState

DatePickerState

DateRangePickerState

snowystinger left a comment • edited Loading

Choose a reason for hiding this comment

ktabors commented Oct 18, 2022 • edited Loading

reidbarber left a comment

Choose a reason for hiding this comment

adobe-bot commented Oct 26, 2022

adobe-bot commented Oct 26, 2022

API Changes

@react-stately/datepicker

DateFieldState

DatePickerState

DateRangePickerState

ethangclark commented Oct 27, 2022

snowystinger commented Oct 27, 2022

devongovett Oct 27, 2022

Choose a reason for hiding this comment

ktabors commented Oct 5, 2022 •

edited

Loading

ktabors Oct 11, 2022 •

edited

Loading

snowystinger left a comment •

edited

Loading

ktabors commented Oct 18, 2022 •

edited

Loading