Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

fix(dropdown): fix tab selection and blur reset #1118

Merged
merged 6 commits into from
Mar 29, 2019

Conversation

silviuaavram
Copy link
Collaborator

@silviuaavram silviuaavram commented Mar 27, 2019

This PR aims to fix 2 issues:

  1. On multiple selection and search, using Tab (and Shift+Tab) to select an item will keep the focus on the dropdown, unless the tabInMultipleSelection is set to true.
  2. On single search, will prevent default Downshift reset behavior when there is a blur on input text.

These 2 issues should also be addressed in Downshift but we need them now, so will patch them now like this and will follow-up in Downshift later. Downshift should make sure that selection on blur works (now it just resets on blur). Also I am planning to add multiple selection to Downshift in the future.

Please provide input about implementation and naming.

Should close #1101.

Copy link
Contributor

@mnajdova mnajdova 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 looks good, but I would suggest adding some unit tests for this. Also, please update the changelog as well!

@codecov
Copy link

codecov bot commented Mar 27, 2019

Codecov Report

Merging #1118 into master will decrease coverage by 0.05%.
The diff coverage is 14.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1118      +/-   ##
==========================================
- Coverage   82.05%   81.99%   -0.06%     
==========================================
  Files         724      724              
  Lines        8619     8626       +7     
  Branches     1234     1236       +2     
==========================================
+ Hits         7072     7073       +1     
- Misses       1530     1536       +6     
  Partials       17       17
Impacted Files Coverage Δ
...ackages/react/src/components/Dropdown/Dropdown.tsx 53.38% <14.28%> (-0.76%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ab73163...5d085f9. Read the comment docs.

packages/react/src/components/Dropdown/Dropdown.tsx Outdated Show resolved Hide resolved
packages/react/src/components/Dropdown/Dropdown.tsx Outdated Show resolved Hide resolved
@@ -185,6 +185,9 @@ export interface DropdownProps extends UIComponentProps<DropdownProps, DropdownS
/** Sets search query value (controlled mode). */
searchQuery?: string

/** When selecting an element with Tab, focus stays on the dropdown by default. If true, the focus will jump to next/previous element in DOM. */
tabInMultipleSelection?: boolean
Copy link
Collaborator

Choose a reason for hiding this comment

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

tabInMultipleSelection is not very descriptive; what about

  • moveFocusOnTab
  • moveFocusOnTabKey
  • moveFocusOnTabPress
  • moveFocusOnTabKeyPress

Or use other verbs instead of move, like release or yield.

What do you think?
@mnajdova @silviuavram

Copy link
Collaborator

@bmdalex bmdalex left a comment

Choose a reason for hiding this comment

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

forgot this is something that needs to get in so I'm not blocking this PR, but pls consider the comments, especially the ones concerning changing the name of the props as we want to avoid having breaking changes

@bmdalex bmdalex dismissed their stale review March 28, 2019 11:11

forgot this is something that needs to get in so I'm not blocking this PR, but pls consider the comments, especially the ones concerning changing the name of the props as we want to avoid having breaking changes

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

Successfully merging this pull request may close these issues.

Dropdown search single selection: selected option is cleared after TAB
4 participants