InputChipsBase: fix click/mousedown/focus issues (#1305)

Author: mdodgelooker

CHANGELOG.md

@@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- `SelectMulti` and `InputChips` issues clicking on icons and chips
 - `Select` display value now properly updates when the option label for the current value changes
 - `InputChips` and `SelectMulti` overflow when a fixed height is used
 - `InputChips` and `SelectMulti` long values breaking out of the input

packages/components/src/Form/Inputs/AdvancedInputControls.tsx

@@ -29,16 +29,17 @@ import flatMap from 'lodash/flatMap'
 import tail from 'lodash/tail'
 import compact from 'lodash/compact'
 import { Icon } from '../../Icon'
-import { InputSearchControls } from './InputSearch/InputSearchControls'
+import {
+  InputSearchControls,
+  InputSearchControlsProps,
+} from './InputSearch/InputSearchControls'
 
-interface AdvancedInputControlsProps {
+export interface AdvancedInputControlsProps
+  extends Omit<InputSearchControlsProps, 'height' | 'showClear'> {
   validationType?: 'error'
   renderSearchControls?: boolean
   isVisibleOptions?: boolean
   hasOptions?: boolean
-  disabled?: boolean
-  onClear: () => void
-  summary?: string
 }
 
 // inserts a divider line between each control element (item1 | item2 | item3)
@@ -50,11 +51,10 @@ const intersperseDivider = (children: ReactElement[]) =>
 export const AdvancedInputControls: FC<AdvancedInputControlsProps> = ({
   validationType,
   renderSearchControls,
-  onClear,
   disabled,
   isVisibleOptions,
-  summary,
   hasOptions = true,
+  ...rest
 }) => {
   const children = intersperseDivider(
     compact([
@@ -70,10 +70,9 @@ export const AdvancedInputControls: FC<AdvancedInputControlsProps> = ({
       renderSearchControls && (
         <InputSearchControls
           key="search-controls"
-          onClear={onClear}
           showClear={true}
           disabled={disabled}
-          summary={summary}
+          {...rest}
         />
       ),
       hasOptions && (

packages/components/src/Form/Inputs/InputChips/InputChips.test.tsx

@@ -148,6 +148,18 @@ tag2`
     expect(onChangeMock).toHaveBeenCalledWith([])
   })
 
+  test('all values are removed by clicking clear field', () => {
+    const onChangeMock = jest.fn()
+    renderWithTheme(
+      <InputChips onChange={onChangeMock} values={['tag1', 'tag2']} />
+    )
+    const clear = screen.getByText('Clear Field')
+
+    fireEvent.click(clear)
+    expect(onChangeMock).toHaveBeenCalledTimes(1)
+    expect(onChangeMock).toHaveBeenCalledWith([])
+  })
+
   test('values are removed by clicking remove on the chip', () => {
     const onChangeMock = jest.fn()
     renderWithTheme(<InputChips onChange={onChangeMock} values={['tag1']} />)
@@ -262,4 +274,57 @@ tag2`
       expect(onChangeMock).not.toHaveBeenCalled()
     })
   })
+
+  test('mousedown on a chip does not focus the inner input', () => {
+    const rafSpy = jest
+      .spyOn(window, 'requestAnimationFrame')
+      .mockImplementation((cb: any) => cb())
+
+    renderWithTheme(
+      <InputChips
+        onChange={() => null}
+        values={['foo', 'bar']}
+        placeholder="type here"
+      />
+    )
+
+    const chip = screen.getByText('bar')
+    const deleteButton = screen.getAllByText('Delete')[0]
+    const input = screen.getByPlaceholderText('type here')
+
+    fireEvent.mouseDown(chip)
+    expect(document.activeElement).not.toEqual(input)
+
+    // Focus should move _after_ delete button is clicked
+    fireEvent.click(deleteButton)
+    expect(document.activeElement).toEqual(input)
+
+    rafSpy.mockRestore()
+  })
+
+  test('mousedown on clear button does not focus the inner input', () => {
+    const rafSpy = jest
+      .spyOn(window, 'requestAnimationFrame')
+      .mockImplementation((cb: any) => cb())
+
+    renderWithTheme(
+      <InputChips
+        onChange={() => null}
+        values={['foo', 'bar']}
+        placeholder="type here"
+      />
+    )
+
+    const clear = screen.getByText('Clear Field')
+    const input = screen.getByPlaceholderText('type here')
+
+    fireEvent.mouseDown(clear)
+    expect(document.activeElement).not.toEqual(input)
+
+    // Focus should move _after_ clear button is clicked
+    fireEvent.click(clear)
+    expect(document.activeElement).toEqual(input)
+
+    rafSpy.mockRestore()
+  })
 })

packages/components/src/Form/Inputs/InputChips/InputChipsBase.tsx

@@ -23,13 +23,21 @@
  SOFTWARE.
 
  */
-import React, { FormEvent, forwardRef, KeyboardEvent, Ref } from 'react'
+import React, {
+  FormEvent,
+  forwardRef,
+  KeyboardEvent,
+  MouseEvent,
+  Ref,
+  useRef,
+} from 'react'
 import styled, { css } from 'styled-components'
 import { MaxHeightProps } from 'styled-system'
 import { Chip } from '../../../Chip'
 import { inputHeight } from '../height'
 import { InputTextContent, InputText, InputTextBaseProps } from '../InputText'
 import { AdvancedInputControls } from '../AdvancedInputControls'
+import { useForkedRef } from '../../../utils'
 
 export interface InputChipsInputControlProps {
   /**
@@ -93,11 +101,25 @@ export const InputChipsBaseInternal = forwardRef(
       removeOnBackspace = true,
       ...props
     }: InputChipsBaseProps & InputChipsInputControlProps,
-    ref: Ref<HTMLInputElement>
+    forwardedRef: Ref<HTMLInputElement>
   ) => {
+    const internalRef = useRef<HTMLInputElement>(null)
+    const ref = useForkedRef(forwardedRef, internalRef)
+
+    // Prevent the default InputText behavior of moving focus to the internal input just after mousedown
+    // on Chip and clear button and instead focus after onChipDelete / onClear
+    // If mousedown/click is elsewhere on Chip, don't move focus b/c user is trying to select the Chip itself
+    function stopPropagation(e: MouseEvent) {
+      e.stopPropagation()
+    }
+    function focusInput() {
+      internalRef.current && internalRef.current.focus()
+    }
+
     function handleDeleteChip(value: string) {
       const newValues = values.filter((v) => value !== v)
       onChange(newValues)
+      focusInput()
     }
 
     function handleKeyDown(e: KeyboardEvent<HTMLInputElement>) {
@@ -112,14 +134,20 @@ export const InputChipsBaseInternal = forwardRef(
       onChange([])
       onInputChange('')
       onClear && onClear()
+      focusInput()
     }
 
     const chips = values.map((value) => {
       function onChipDelete() {
         handleDeleteChip(value)
       }
       return (
-        <Chip onDelete={onChipDelete} key={value}>
+        <Chip
+          onDelete={onChipDelete}
+          onMouseDown={stopPropagation}
+          onClick={stopPropagation}
+          key={value}
+        >
           {value}
         </Chip>
       )
@@ -143,6 +171,7 @@ export const InputChipsBaseInternal = forwardRef(
               disabled={disabled}
               summary={summary}
               hasOptions={hasOptions}
+              onMouseDown={stopPropagation}
             />
           )
         }

packages/components/src/Form/Inputs/InputSearch/InputSearchControls.tsx

@@ -24,14 +24,16 @@
 
  */
 
+import { CompatibleHTMLProps } from '@looker/design-tokens'
 import omit from 'lodash/omit'
 import React, { forwardRef, MouseEvent, Ref } from 'react'
 import styled from 'styled-components'
 import { Box, Space } from '../../../Layout'
 import { IconButton } from '../../../Button'
 import { Text } from '../../../Text'
 
-export interface InputSearchControlsProps {
+export interface InputSearchControlsProps
+  extends CompatibleHTMLProps<HTMLDivElement> {
   summary?: string
   showClear: boolean
   onClear: (e: MouseEvent<HTMLButtonElement>) => void