Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EuiFormRow and EuiDatePickerRange focus behaviour #6128

Closed
anthonyhastings opened this issue Aug 11, 2022 · 2 comments · Fixed by #6136
Closed

EuiFormRow and EuiDatePickerRange focus behaviour #6128

anthonyhastings opened this issue Aug 11, 2022 · 2 comments · Fixed by #6136

Comments

@anthonyhastings
Copy link
Contributor

Hi team,

I've encountered an issue when using EuiDatePickerRange as the nested component within EuiFormRow.

Reproducible Example:
https://codesandbox.io/s/mystifying-frost-80hfqv

Reproduction Steps:

  1. Focus into either the start or end date control which adds focus state to the row component and highlights the label.
  2. Click outside the controls to trigger blur and observe the row components focus state disappears as expected.
  3. Focus into either date picker which adds focus state to the row component.
  4. Choose a date and notice that when the date picker closes, the focus state of the row component remains.

Initial Investigation:
EuiFormRow expects a single child component, which it clones and supplies with monkey patched onBlur and onFocus events. It uses these monkey patched handlers to manage internal focus state, which is in turn used to show the focused state on the field label.

EuiFormRow expects it's child component to either explicitly or implicitly apply the onBlur and onFocus handlers onto a form control contained within which would keep the focus state in sync with any interactions on the form element. When the row component has a child such as EuiDatePicker or EuiFieldText things work fine as those components implicitly spread the props onto the input.

Whenever EuiDatePickerRange is used as the child component things get sketchy; this component implicitly applies those handlers onto a wrapper div element and not an actual form element. I'm surprised anything here actually works as to my own knowledge, only a div that had tabindex would actually be able to trigger blur and focus events. When the datepickers handle the selection of a date, no blur seems to emit from the div element and the row components state becomes frozen.

Potential Solution:
Locally I have a potential fix - I updated EuiDatePickerRange component to no longer spread onBlur and onFocus props onto the div container element. I then apply those handlers onto both the start and end date components (ensuring if they have handlers of their own that they get monkey patched). This means the handlers go to the form controls themselves, and now the rows focus state is in sync with the date pickers as they open/close/select values.

I'd be happy to PR properly if it sounds feasible!

Many Thanks

@cee-chen
Copy link
Member

cee-chen commented Aug 12, 2022

Thanks for the super thorough explanation of what's happening @anthonyhastings! You're totally correct, the onFocus and onBlur coming from EuiFormRow should be applied to the individual start/end controls instead of spread to the div. What you describe sounds like exactly what I'd do to fix the problem as well. I'd love to see a PR for sure!! 🙌

@anthonyhastings
Copy link
Contributor Author

Sounds great @constancecchen - I've opened up #6136 which addresses the issue but I'd love some guidance on what else needs done to get this over the line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants