fix: Let wave app programmatically change value and choices for combobox

py/examples/combobox.py

@@ -20,13 +20,13 @@ async def serve(q: Q):
         ]
     else:
         q.page['example'] = ui.form_card(box='1 1 4 10', items=[
-            ui.combobox(name='combobox', label='Enter or choose a color', placeholder='Color...', value='Blue',
+            ui.combobox(name='combobox', label='Enter or choose a color', placeholder='Color...', value='Black',
                         choices=combobox_choices),
-            ui.combobox(name='combobox_required', label='Enter or choose a color', placeholder='Color...', value='Blue',
+            ui.combobox(name='combobox_required', label='Enter or choose a color', placeholder='Color...', value='Black',
                         choices=combobox_choices, required=True),
-            ui.combobox(name='combobox_disabled', label='Enter or choose a color', placeholder='Color...', value='Blue',
+            ui.combobox(name='combobox_disabled', label='Enter or choose a color', placeholder='Color...', value='Black',
                         choices=combobox_choices, disabled=True),
-            ui.combobox(name='combobox_error', label='Enter or choose a color', placeholder='Color...', value='Blue',
+            ui.combobox(name='combobox_error', label='Enter or choose a color', placeholder='Color...', value='Black',
                         choices=combobox_choices, error='This combobox has an error!'),
             ui.combobox(name='combobox_multivalued', label='Enter or choose a color (multi select)', placeholder='Color...', values=['Black', 'Magenta'],
                         choices=combobox_choices),

ui/src/combobox.test.tsx

@@ -19,7 +19,7 @@ import { Combobox, XCombobox } from './combobox'
 import { wave } from './ui'
 
 const name = 'combobox'
-const comboboxProps: Combobox = { name, choices: ['Choice1', 'Choice2', 'Choice3'] }
+const comboboxProps: Combobox = { name, choices: ['A', 'B', 'C'] }
 describe('Combobox.tsx', () => {
   beforeEach(() => { wave.args[name] = null })
 
@@ -33,47 +33,46 @@ describe('Combobox.tsx', () => {
     expect(wave.args[name]).toBeNull()
   })
   it('Sets args - init - value specified', () => {
-    render(<XCombobox model={{ ...comboboxProps, value: 'Test' }} />)
-    expect(wave.args[name]).toBe('Test')
+    render(<XCombobox model={{ ...comboboxProps, value: 'A' }} />)
+    expect(wave.args[name]).toBe('A')
   })
 
   it('Sets args - selection', () => {
     const { getByRole, getByText } = render(<XCombobox model={{ ...comboboxProps }} />)
     fireEvent.click(getByRole('presentation', { hidden: true }))
-    fireEvent.click(getByText('Choice1'))
+    fireEvent.click(getByText('A'))
 
-    expect(wave.args[name]).toBe('Choice1')
+    expect(wave.args[name]).toBe('A')
   })
 
   it('Sets args - multiple selection', () => {
     const { getByRole, getByText } = render(<XCombobox model={{ ...comboboxProps, values: [] }} />)
     fireEvent.click(getByRole('presentation', { hidden: true }))
-    fireEvent.click(getByText('Choice1'))
-    fireEvent.click(getByText('Choice2'))
+    fireEvent.click(getByText('A'))
+    fireEvent.click(getByText('B'))
 
-    expect(wave.args[name]).toEqual(['Choice1', 'Choice2'])
+    expect(wave.args[name]).toEqual(['A', 'B'])
   })
 
   it('Types new option', () => {
-    const initialValues = ['Choice1']
-    const { getByRole } = render(<XCombobox model={{ ...comboboxProps, values: initialValues }} />)
-
-    expect(wave.args[name]).toEqual(initialValues)
-    userEvent.type(getByRole('combobox'), 'Choice4{Enter}')
-    expect(wave.args[name]).toEqual(['Choice1', 'Choice4'])
+    const { getByRole } = render(<XCombobox model={{ ...comboboxProps, values: ['A'] }} />)
+    expect(wave.args[name]).toEqual(['A'])
+
+    userEvent.type(getByRole('combobox'), 'D{Enter}')
+    expect(wave.args[name]).toEqual(['A', 'D'])
   })
 
   it('Unselects every option and types a new one', () => {
-    const initialValues = ['Choice1', 'Choice2']
-    const { getByText, getByRole } = render(<XCombobox model={{ ...comboboxProps, values: initialValues }} />)
+    const { getByText, getByRole } = render(<XCombobox model={{ ...comboboxProps, values: ['A', 'B'] }} />)
+    expect(wave.args[name]).toEqual(['A', 'B'])
 
-    expect(wave.args[name]).toEqual(initialValues)
     fireEvent.click(getByRole('presentation', { hidden: true }))
-    fireEvent.click(getByText('Choice1'))
-    fireEvent.click(getByText('Choice2'))
+    fireEvent.click(getByText('A'))
+    fireEvent.click(getByText('B'))
     expect(wave.args[name]).toEqual([])
-    userEvent.type(getByRole('combobox'), 'Choice4{Enter}')
-    expect(wave.args[name]).toEqual(['Choice4'])
+
+    userEvent.type(getByRole('combobox'), 'D{Enter}')
+    expect(wave.args[name]).toEqual(['D'])
   })
 
   it('Calls sync when trigger is on', () => {
@@ -82,17 +81,130 @@ describe('Combobox.tsx', () => {
     const { getByRole, getByText } = render(<XCombobox model={{ ...comboboxProps, trigger: true }} />)
 
     fireEvent.click(getByRole('presentation', { hidden: true }))
-    fireEvent.click(getByText('Choice1'))
+    fireEvent.click(getByText('A'))
 
     expect(pushMock).toHaveBeenCalled()
   })
 
   it('Selects and unselects a user typed option', () => {
     const { getByRole, getByText } = render(<XCombobox model={{ ...comboboxProps, values: [] }} />)
-    userEvent.type(getByRole('combobox'), 'Choice4{Enter}')
-    expect(wave.args[name]).toEqual(['Choice4'])
+    userEvent.type(getByRole('combobox'), 'D{Enter}')
+    expect(wave.args[name]).toEqual(['D'])
     fireEvent.click(getByRole('presentation', { hidden: true }))
-    fireEvent.click(getByText('Choice4'))
+    fireEvent.click(getByText('D'))
     expect(wave.args[name]).toEqual([])
   })
+
+  describe('Prop changes - update values dynamically from Wave app', () => {
+    it('Updates combobox value when "value" prop changes', () => {
+      const { getByRole, getByText, rerender } = render(<XCombobox model={{ ...comboboxProps, value: 'A' }} />)
+      expect(getByRole('combobox')).toHaveValue('A')
+      expect(wave.args[name]).toEqual('A')
+
+      rerender(<XCombobox model={{ ...comboboxProps, value: 'B' }} />)
+      expect(getByRole('combobox')).toHaveValue('B')
+      expect(wave.args[name]).toEqual('B')
+
+      fireEvent.click(getByRole('presentation', { hidden: true }))
+      fireEvent.click(getByText('C'))
+      expect(getByRole('combobox')).toHaveValue('C')
+      expect(wave.args[name]).toEqual('C')
+
+      rerender(<XCombobox model={{ ...comboboxProps, value: 'B' }} />)
+      expect(getByRole('combobox')).toHaveValue('B')
+      expect(wave.args[name]).toEqual('B')
+    })
+
+    it('Types new option and then updates combobox value when "value" prop changes', () => {
+      const { getByRole, getByText, rerender } = render(<XCombobox model={{ ...comboboxProps, value: 'A' }} />)
+      expect(getByRole('combobox')).toHaveValue('A')
+      expect(wave.args[name]).toEqual('A')
+
+      fireEvent.click(getByRole('presentation', { hidden: true }))
+      fireEvent.click(getByText('B'))
+      fireEvent.blur(getByRole('presentation', { hidden: true }))
+      userEvent.type(getByRole('combobox'), 'B{Enter}')
+      expect(getByRole('combobox')).toHaveValue('BB')
+      expect(wave.args[name]).toEqual('BB')
+
+      rerender(<XCombobox model={{ ...comboboxProps, value: 'C' }} />)
+      expect(getByRole('combobox')).toHaveValue('C')
+      expect(wave.args[name]).toEqual('C')
+    })
+
+    it('Types new option and then updates combobox value when "values" prop changes - multi select', () => {
+      const { getByRole, getByText, rerender } = render(<XCombobox model={{ ...comboboxProps, values: ['A', 'B'] }} />)
+      expect(getByRole('combobox')).toHaveValue('A, B')
+
+      rerender(<XCombobox model={{ ...comboboxProps, values: ['C'] }} />)
+      expect(getByRole('combobox')).toHaveValue('C')
+
+      fireEvent.click(getByRole('presentation', { hidden: true }))
+      fireEvent.click(getByText('B'))
+      fireEvent.blur(getByRole('presentation', { hidden: true }))
+      expect(getByRole('combobox')).toHaveValue('B, C')
+
+      rerender(<XCombobox model={{ ...comboboxProps, values: ['A', 'B'] }} />)
+      expect(getByRole('combobox')).toHaveValue('A, B')
+    })
+
+    it('Updates "choices" single select', () => {
+      const { getByRole, getAllByRole, rerender } = render(<XCombobox model={{ ...comboboxProps, value: 'A' }} />)
+      fireEvent.click(getByRole('presentation', { hidden: true }))
+      expect(getAllByRole('option')).toHaveLength(3)
+
+      rerender(<XCombobox model={{ ...comboboxProps, choices: ['A', 'B'] }} />)
+      expect(getAllByRole('option')).toHaveLength(2)
+    })
+
+    it('Updates "choices" prop - multi select', () => {
+      const { getByRole, getAllByRole, rerender } = render(<XCombobox model={{ ...comboboxProps, values: ['A'] }} />)
+      fireEvent.click(getByRole('presentation', { hidden: true }))
+      expect(getAllByRole('option')).toHaveLength(3)
+
+      rerender(<XCombobox model={{ ...comboboxProps, choices: ['A', 'B'] }} />)
+      fireEvent.click(getByRole('presentation', { hidden: true }))
+      expect(getAllByRole('option')).toHaveLength(2)
+    })
+
+    it('Updates "choices" prop and "value" prop to value different than the initial one', () => {
+      const { getByRole, rerender } = render(<XCombobox model={{ ...comboboxProps, value: 'A' }} />)
+      expect(wave.args[name]).toEqual('A')
+      expect(getByRole('combobox')).toHaveValue('A')
+
+      rerender(<XCombobox model={{ ...comboboxProps, choices: ['A', 'B'], value: 'B' }} />)
+      expect(getByRole('combobox')).toHaveValue('B')
+    })
+
+    it('Clears combobox if "value" is not included in choices', () => {
+      const { getByRole, rerender } = render(<XCombobox model={{ ...comboboxProps, value: 'A' }} />)
+      expect(wave.args[name]).toEqual('A')
+      expect(getByRole('combobox')).toHaveValue('A')
+
+      rerender(<XCombobox model={{ ...comboboxProps, value: 'D' }} />)
+      expect(getByRole('combobox')).toHaveValue('')
+      expect(wave.args[name]).toBeNull()
+    })
+
+    it('Clears combobox if none of the value in "values" are included in "choices"', () => {
+      const { getByRole, rerender } = render(<XCombobox model={{ ...comboboxProps, values: ['A'] }} />)
+      expect(wave.args[name]).toEqual(['A'])
+      expect(getByRole('combobox')).toHaveValue('A')
+      expect(wave.args[name]).toEqual(['A'])
+
+      rerender(<XCombobox model={{ ...comboboxProps, values: ['D'] }} />)
+      expect(getByRole('combobox')).toHaveValue('')
+      expect(wave.args[name]).toEqual([])
+    })
+
+    it('Selects only "values" present in "choices" - intersection of "values" and "choices"', () => {
+      const { getByRole, rerender } = render(<XCombobox model={{ ...comboboxProps, values: ['A', 'B'] }} />)
+      expect(wave.args[name]).toEqual(['A', 'B'])
+      expect(getByRole('combobox')).toHaveValue('A, B')
+
+      rerender(<XCombobox model={{ ...comboboxProps, values: ['B', 'D'] }} />)
+      expect(getByRole('combobox')).toHaveValue('B')
+      expect(wave.args[name]).toEqual(['B'])
+    })
+  })
 })

ui/src/combobox.tsx

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 import * as Fluent from '@fluentui/react'
+import { IComboBox, IComboBoxOption } from '@fluentui/react'
 import { B, Id, S, U } from 'h2o-wave'
 import React from 'react'
 import { wave } from './ui'
@@ -59,54 +60,108 @@ export interface Combobox {
   required?: B
 }
 
-export const
-  XCombobox = ({ model: m }: { model: Combobox }) => {
-    const
-      isMultiValued = !!m.values,
-      [text, setText] = React.useState(m.value),
-      [options, setOptions] = React.useState((m.choices || []).map((text): Fluent.IComboBoxOption => ({ key: text, text }))),
-      [selected, setSelected] = React.useState<Fluent.IComboBoxOption[]>((m.values || []).map(text => ({ key: text, text }))),
-      selectOpt = (option: Fluent.IComboBoxOption) => {
-        setSelected(items => {
-          const result = option.selected ? [...items, option] : items.filter(item => item.key !== option.key)
-          wave.args[m.name] = result.map(item => item.text)
-          return result
-        })
-      },
-      onChange = (_e: React.FormEvent<Fluent.IComboBox>, option?: Fluent.IComboBoxOption, _index?: U, value?: S) => {
-        if (!option && value) {
-          const opt: Fluent.IComboBoxOption = { key: value, text: value }
-          setOptions((prevOptions = []) => [...prevOptions, opt])
-          selectOpt({...opt, selected: true})
-        }
-        if (option && isMultiValued) {
-          selectOpt(option)
-        }
-        else {
-          const v = option?.text || value || ''
-          wave.args[m.name] = v
-          setText(v)
-        }
-        if (m.trigger) wave.push()
+export const XCombobox = ({ model: m }: { model: Combobox }) => !m.values ? <ComboboxSingleSelect model={m} /> : <ComboboxMultiSelect model={m} />
+
+const ComboboxSingleSelect = ({ model: m }: { model: Omit<Combobox, 'values'> }) => {
+  const
+    [options, setOptions] = useOptions(m.choices),
+    [selected, setSelected] = React.useState<S | null>(m.value ?? null),
+    skipUseEffectForValue = React.useRef(false),
+    onChange = (_e: React.FormEvent<IComboBox>, option?: IComboBoxOption, _index?: U, value?: S) => {
+      if (!option && value) setOptions((prevOptions = []) => [...prevOptions, { key: value, text: value }])
+
+      const v = option?.text || value || ''
+      setSelected(v)
+
+      wave.args[m.name] = v
+      if (m.trigger) wave.push()
+
+      // Hacky: Ensure that next dynamically change to value from Wave app will trigger the useEffect
+      m.value = v
+      // The line above will trigger the useEffect for change in m.value and we need to skip it
+      skipUseEffectForValue.current = true
+    }
+
+  React.useEffect(() => {
+    wave.args[m.name] = (selected && options.map(o => String(o.key)).includes(selected || '')) ? selected : null
+  }, [m.name, m.value, options, selected])
+
+  // Select value when "value" is set from Wave App dynamically
+  React.useEffect(() => {
+    if (!skipUseEffectForValue.current) setSelected(m.value!)
+    skipUseEffectForValue.current = false
+  }, [m.value])
+
+  return (
+    <Fluent.ComboBox
+      data-test={m.name}
+      label={m.label}
+      placeholder={m.placeholder}
+      options={options}
+      required={m.required}
+      selectedKey={selected}
+      allowFreeform
+      disabled={m.disabled}
+      autoComplete="on"
+      errorMessage={m.error}
+      onChange={onChange}
+    />
+  )
+}
+
+const ComboboxMultiSelect = ({ model: m }: { model: Omit<Combobox, 'value'> }) => {
+  const
+    [options, setOptions] = useOptions(m.choices),
+    [selected, setSelected] = React.useState<S[]>(m.values ?? []),
+    selectOpt = (option: IComboBoxOption) => {
+      setSelected(keys => {
+        const result = option.selected ? [...keys, String(option.key)] : (keys as S[]).filter(key => key !== option.key)
+        wave.args[m.name] = result
+        return result
+      })
+    },
+    onChange = (_e: React.FormEvent<IComboBox>, option?: IComboBoxOption, _index?: U, value?: S) => {
+      if (!option && value) {
+        const opt: IComboBoxOption = { key: value, text: value }
+        setOptions((prevOptions = []) => [...prevOptions, opt])
+        selectOpt({...opt, selected: true})
+      }
+      if (option) {
+        selectOpt(option)
       }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-    React.useEffect(() => { wave.args[m.name] = m?.value || m?.values || null }, [])
+      if (m.trigger) wave.push()
+    }
+
+  React.useEffect(() => {
+      const intersection = options.map(o => String(o.key)).filter(choice => selected?.includes(choice))
+      wave.args[m.name] = intersection ?? null
+    }, [m.name, m.values, options, selected])
+
+  React.useEffect(() => setSelected(m.values || []), [m.values])
 
-    return (
-      <Fluent.ComboBox
-        data-test={m.name}
-        label={m.label}
-        placeholder={m.placeholder}
-        options={options}
-        required={m.required}
-        multiSelect={isMultiValued}
-        selectedKey={isMultiValued ? selected.map(s => s.key as S) : undefined}
-        text={isMultiValued ? undefined : text}
-        allowFreeform
-        disabled={m.disabled}
-        autoComplete="on"
-        errorMessage={m.error}
-        onChange={onChange}
-      />
-    )
-  }
+  return (
+    <Fluent.ComboBox
+      data-test={m.name}
+      label={m.label}
+      placeholder={m.placeholder}
+      options={options}
+      required={m.required}
+      multiSelect={true}
+      selectedKey={selected}
+      allowFreeform
+      disabled={m.disabled}
+      autoComplete="on"
+      errorMessage={m.error}
+      onChange={onChange}
+    />
+  )
+}
+
+const useOptions = (choices: S[] = []): [Fluent.IComboBoxOption[], React.Dispatch<React.SetStateAction<Fluent.IComboBoxOption[]>>]  => {
+  const mapChoices = React.useMemo(() => (choices || []).map((text): Fluent.IComboBoxOption => ({ key: text, text })), [choices])
+  const [options, setOptions] = React.useState(mapChoices)
+
+  React.useEffect(() => { setOptions(mapChoices) }, [choices, mapChoices])
+
+  return [options, setOptions]
+}