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

fix: add ActionType.CLEAR_FOCUS to indicate a user has left the field #902

Merged
merged 5 commits into from
Feb 22, 2021

Conversation

christopherhuii
Copy link
Contributor

@christopherhuii christopherhuii commented Feb 18, 2021

Summary

The ComboBox has issues with "losing" its focus when clicking outside of the component.

Screen.Recording.2021-02-18.at.1.49.43.PM.mov

A related issue is that the FocusMode is still set to FocusMode.Input even after the user has left the component.

This PR allows the user to:

  1. Unfocus from the ComboBox when they click outside
  2. Resets the FocusMode to None when they click outside

I achieved this by creating a new ActionType called CLEAR_FOCUS to indicate when a user has clicked outside of the combo box. I am open to suggestions on the name. This action is different from ActionType.CLEAR because CLEAR empties the input, whereas CLEAR_FOCUS indicates the user has left the field.

Related Issues or PRs

Fixes #901

How To Test

  1. Open Storybook
  2. Focus ComboBox by opening its options or typing in the input
  3. Click outside of the component and the input should lose focus.

Screenshots (optional)

After Fix:

Screen.Recording.2021-02-18.at.2.13.02.PM.mov

@christopherhuii christopherhuii added the status: wip Work in progress - not ready for code review label Feb 18, 2021
@christopherhuii christopherhuii changed the title [WIP][ComboBox] clear focusMode when ActionTypes.CLEAR is dispatched fix: clear focusMode when ActionTypes.CLEAR is dispatched Feb 18, 2021
@trussworks-infra-zz
Copy link

trussworks-infra-zz commented Feb 18, 2021

Warnings
⚠️ This PR does not include changes to storybook, even though it affects component code.

Generated by 🚫 dangerJS against 727ed1f

@christopherhuii christopherhuii changed the title fix: clear focusMode when ActionTypes.CLEAR is dispatched fix: add ActionType.CLEAR_FOCUS to indicate a user has left the field Feb 18, 2021
@christopherhuii christopherhuii marked this pull request as ready for review February 18, 2021 22:20
@christopherhuii christopherhuii removed the status: wip Work in progress - not ready for code review label Feb 18, 2021
@christopherhuii christopherhuii requested a review from jim February 18, 2021 22:20
@christopherhuii
Copy link
Contributor Author

Maybe ActionType.BLUR is a better name?

suzubara
suzubara previously approved these changes Feb 22, 2021
Copy link
Contributor

@suzubara suzubara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes & testing the component in Storybook look good to me! If they have time, I might look for a review from @haworku or @jim since they worked on the original component, but I feel pretty confident that this is an improvement.

Maybe ActionType.BLUR is a better name?

I agree!

Copy link
Contributor

@jim jim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me! Thanks for digging into this component and fixing this issue.

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 this pull request may close these issues.

[fix] ComboBox forces focus when another field is changed
4 participants