Repository: uber-web/baseui
Pull request #668 (2018-12-18 22:38:10)

fix(select): fix blur and close select when double clicked
`onBlur` doesn't really work for us since the actual `<input />` is not visible/clickable so essentially any click (on the select component or outside) triggers the `onBlur` event. 

I think the `onBlur` should be triggered only when the select component is focused and it's transitioning into non-focused state (click outside or an option is selected).

The solution is `document.addEventListener('click',...)` and re-implemeting `onBlur` that way. Definitely not ideal but not sure what else we can do if we don't use native input/select components. One downside is that it doesn't register clicks in different iframes (but that's probably not a big concern?) and also the event parameter being passed is different (target = what user clicked on vs what's being "blurred").

With that, we also want to change the behavior of select - when it's double clicked it should close but stay focused.

**Before:**
![before](https://user-images.githubusercontent.com/1387913/50187206-3f4e6200-02d2-11e9-8a63-2195f7afd407.gif)

**After:**
![after](https://user-images.githubusercontent.com/1387913/50187210-44abac80-02d2-11e9-9e89-c61ddda4934d.gif)

nadiia: Need to add keyboard support for it as well. Currently when I tab fro the select it still maintenance focused styles.
nadiia: The other issue is tabbing out while the dropdown is opened changed the behavior. See the screen cast below.

Was:
![sel1](https://user-images.githubusercontent.com/1920399/50189123-e209df00-02d8-11e9-99d1-682f8d134237.gif)

Became:
![sel2](https://user-images.githubusercontent.com/1920399/50189143-efbf6480-02d8-11e9-8dfe-052b177f26bd.gif)


tajo: > The other issue is tabbing out while the dropdown is opened changed the behavior.

Fixed.

Patch:
diff --git a/src/select/select.js b/src/select/select.js
index eafb47b4c..0c1e20893 100644
--- a/src/select/select.js
+++ b/src/select/select.js
@@ -53,6 +53,7 @@ class Select extends React.Component<PropsT, SelectStateT> {
   dragging: boolean;
   focusAfterClear: boolean;
   openAfterFocus: boolean;
+  isValueJustSelected: boolean;
 
   state = {
     inputValue: '',
@@ -66,7 +67,6 @@ class Select extends React.Component<PropsT, SelectStateT> {
       this.focus();
     }
   }
-
   componentDidUpdate(prevProps: PropsT, prevState: SelectStateT) {
     if (prevState.isOpen !== this.state.isOpen) {
       this.toggleTouchOutsideEvent(this.state.isOpen);
@@ -75,6 +75,11 @@ class Select extends React.Component<PropsT, SelectStateT> {
         : this.props.onClose;
       handler && handler();
     }
+    if (prevState.isFocused !== this.state.isFocused && this.state.isFocused) {
+      if (__BROWSER__) {
+        document.addEventListener('click', this.handleClickOutside);
+      }
+    }
   }
 
   componentWillUnmount() {
@@ -130,6 +135,18 @@ class Select extends React.Component<PropsT, SelectStateT> {
     this.clearValue(event);
   };
 
+  handleClickOutside = (event: Event) => {
+    if (this.isValueJustSelected) {
+      this.isValueJustSelected = false;
+      return;
+    }
+    const isFocused = this.state.isFocused || this.state.isPseudoFocused;
+    // $FlowFixMe
+    if (isFocused && this.wrapper && !this.wrapper.contains(event.target)) {
+      this.handleInputBlur(event);
+    }
+  };
+
   handleClick = (event: Event) => {
     // If the event was triggered by a mouse click and not the primary
     // button, or if the component is disabled, ignore it
@@ -170,7 +187,7 @@ class Select extends React.Component<PropsT, SelectStateT> {
       // calls to input.focus() that weren't triggered by a click event.
       // Call focus() again here to be safe.
       this.focus();
-      let toOpen = true;
+      let toOpen = !this.state.isOpen;
       // clears the value so that the cursor will be at the end of input when the component re-renders
       if (this.input) this.input.value = '';
       if (this.focusAfterClear) {
@@ -247,7 +264,7 @@ class Select extends React.Component<PropsT, SelectStateT> {
     this.openAfterFocus = false;
   };
 
-  handleInputBlur = (event: SyntheticEvent<HTMLElement>) => {
+  handleInputBlur = (event: Event) => {
     if (this.props.onBlur) {
       this.props.onBlur(event);
     }
@@ -261,6 +278,9 @@ class Select extends React.Component<PropsT, SelectStateT> {
       onBlurredState.inputValue = '';
     }
     this.setState(onBlurredState);
+    if (__BROWSER__) {
+      document.removeEventListener('click', this.handleClickOutside);
+    }
   };
 
   handleInputChange = (event: SyntheticInputEvent<HTMLInputElement>) => {
@@ -291,6 +311,14 @@ class Select extends React.Component<PropsT, SelectStateT> {
           this.setState({isOpen: true});
         }
         break;
+      case 9: // tab
+        this.setState(prevState => ({
+          isPseudoFocused: false,
+          isFocused: false,
+          isOpen: false,
+          inputValue: this.props.onCloseResetsInput ? '' : prevState.inputValue,
+        }));
+        break;
       case 27: // escape
         event.preventDefault();
         if (this.state.isOpen) {
@@ -387,6 +415,9 @@ class Select extends React.Component<PropsT, SelectStateT> {
   }
 
   selectValue = ({item}: {item: OptionT}) => {
+    // NOTE: we check this is in handleClickOutside to not count
+    // menu clicks as outside clicks
+    this.isValueJustSelected = true;
     if (item.disabled) {
       return;
     }
@@ -581,7 +612,6 @@ class Select extends React.Component<PropsT, SelectStateT> {
       'aria-disabled': this.props.disabled || null,
       disabled: this.props.disabled || null,
       inputRef: ref => (this.input = ref),
-      onBlur: this.handleInputBlur,
       onChange: this.handleInputChange,
       onFocus: this.handleInputFocus,
       overrides: {Input: overrides.Input},
@@ -596,7 +626,6 @@ class Select extends React.Component<PropsT, SelectStateT> {
           aria-disabled={this.props.disabled}
           aria-label={this.props['aria-label']}
           aria-labelledby={this.props['aria-labelledby']}
-          onBlur={this.handleInputBlur}
           onFocus={this.handleInputFocus}
           $ref={ref => (this.input = ref)}
           role="combobox"
diff --git a/src/select/types.js b/src/select/types.js
index fe05370c1..719140cc5 100644
--- a/src/select/types.js
+++ b/src/select/types.js
@@ -85,7 +85,7 @@ export type PropsT = {
   maxDropdownHeight: string,
   multi: boolean,
   noResultsMsg: React.Node,
-  onBlur: (e: SyntheticEvent<HTMLElement>) => void,
+  onBlur: (e: Event) => void,
   onBlurResetsInput: boolean,
   onChange: (params: OnChangeParamsT) => void,
   onFocus: (e: SyntheticEvent<HTMLElement>) => void,


Test patch:
diff --git a/src/select/__tests__/__snapshots__/select.test.js.snap b/src/select/__tests__/__snapshots__/select.test.js.snap
index 2d7a45eae..2b87e0c3f 100644
--- a/src/select/__tests__/__snapshots__/select.test.js.snap
+++ b/src/select/__tests__/__snapshots__/select.test.js.snap
@@ -259,7 +259,6 @@ exports[`Select component renders component in search mode and false for multipl
                     aria-labelledby={null}
                     disabled={null}
                     inputRef={[Function]}
-                    onBlur={[Function]}
                     onChange={[Function]}
                     onFocus={[Function]}
                     overrides={
@@ -294,7 +293,6 @@ exports[`Select component renders component in search mode and false for multipl
                       aria-label={null}
                       aria-labelledby={null}
                       disabled={null}
-                      onBlur={[Function]}
                       onChange={[Function]}
                       onFocus={[Function]}
                       required={false}
@@ -310,7 +308,6 @@ exports[`Select component renders component in search mode and false for multipl
                         aria-label={null}
                         aria-labelledby={null}
                         disabled={null}
-                        onBlur={[Function]}
                         onChange={[Function]}
                         onFocus={[Function]}
                         required={false}
@@ -615,7 +612,6 @@ exports[`Select component renders component in search mode and true for multiple
                     aria-labelledby={null}
                     disabled={null}
                     inputRef={[Function]}
-                    onBlur={[Function]}
                     onChange={[Function]}
                     onFocus={[Function]}
                     overrides={
@@ -650,7 +646,6 @@ exports[`Select component renders component in search mode and true for multiple
                       aria-label={null}
                       aria-labelledby={null}
                       disabled={null}
-                      onBlur={[Function]}
                       onChange={[Function]}
                       onFocus={[Function]}
                       required={false}
@@ -666,7 +661,6 @@ exports[`Select component renders component in search mode and true for multiple
                         aria-label={null}
                         aria-labelledby={null}
                         disabled={null}
-                        onBlur={[Function]}
                         onChange={[Function]}
                         onFocus={[Function]}
                         required={false}
@@ -893,7 +887,6 @@ exports[`Select component renders component in select mode and false for multipl
                     aria-labelledby={null}
                     disabled={null}
                     inputRef={[Function]}
-                    onBlur={[Function]}
                     onChange={[Function]}
                     onFocus={[Function]}
                     overrides={
@@ -928,7 +921,6 @@ exports[`Select component renders component in select mode and false for multipl
                       aria-label={null}
                       aria-labelledby={null}
                       disabled={null}
-                      onBlur={[Function]}
                       onChange={[Function]}
                       onFocus={[Function]}
                       required={false}
@@ -944,7 +936,6 @@ exports[`Select component renders component in select mode and false for multipl
                         aria-label={null}
                         aria-labelledby={null}
                         disabled={null}
-                        onBlur={[Function]}
                         onChange={[Function]}
                         onFocus={[Function]}
                         required={false}
@@ -1248,7 +1239,6 @@ exports[`Select component renders component in select mode and true for multiple
                     aria-labelledby={null}
                     disabled={null}
                     inputRef={[Function]}
-                    onBlur={[Function]}
                     onChange={[Function]}
                     onFocus={[Function]}
                     overrides={
@@ -1283,7 +1273,6 @@ exports[`Select component renders component in select mode and true for multiple
                       aria-label={null}
                       aria-labelledby={null}
                       disabled={null}
-                      onBlur={[Function]}
                       onChange={[Function]}
                       onFocus={[Function]}
                       required={false}
@@ -1299,7 +1288,6 @@ exports[`Select component renders component in select mode and true for multiple
                         aria-label={null}
                         aria-labelledby={null}
                         disabled={null}
-                        onBlur={[Function]}
                         onChange={[Function]}
                         onFocus={[Function]}
                         required={false}