Skip to content

Commit

Permalink
chore: Unify value behavior #1154 (#2118)
Browse files Browse the repository at this point in the history
  • Loading branch information
@marek-mihok
marek-mihok committed Jan 15, 2024
1 parent afb3815 commit 3a252a0
Show file tree
Hide file tree
Showing 13 changed files with 378 additions and 138 deletions.
18 changes: 17 additions & 1 deletion ui/src/checklist.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@ import { Checklist, XChecklist } from './checklist'
import { wave } from './ui'

const name = 'checklist'
const checklistProps: Checklist = { name, choices: [{ name: 'Choice1' }, { name: 'Choice2' }, { name: 'Choice3' },] }
const choices = [{ name: 'Choice1' }, { name: 'Choice2' }, { name: 'Choice3' },]
const checklistProps: Checklist = { name, choices }
describe('Checklist.tsx', () => {
beforeEach(() => {
jest.clearAllMocks()
wave.args[name] = null
checklistProps.values = undefined
})

it('Renders data-test attr', () => {
Expand Down Expand Up @@ -64,6 +66,20 @@ describe('Checklist.tsx', () => {
expect(wave.args[name]).toMatchObject(['Choice1'])
})

it('Set correct args when value is cleared twice', () => {
const { getByText, rerender } = render(<XChecklist model={{ ...checklistProps, values: ['Choice1'] }} />)
expect(wave.args[name]).toMatchObject(['Choice1'])

rerender(<XChecklist model={{ ...checklistProps }} />)
expect(wave.args[name]).toMatchObject([])

fireEvent.click(getByText('Choice2').parentElement!)
expect(wave.args[name]).toMatchObject(['Choice2'])

rerender(<XChecklist model={{ ...checklistProps }} />)
expect(wave.args[name]).toMatchObject([])
})

it('Updates choices', () => {
const { rerender, getByText } = render(<XChecklist model={checklistProps} />)
expect(getByText('Choice1')).toBeInTheDocument()
Expand Down
15 changes: 12 additions & 3 deletions ui/src/checklist.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,17 @@ export const
const _choices = choices.map(({ c, selected }) => ({ c, selected: c.disabled ? selected : value }))
setChoices(_choices)
capture(_choices)
m.values = value ? _choices.map(({ c }) => c.name) : []
},
selectAll = () => select(true),
deselectAll = () => select(false),
onChange = (idx: U) => (_e?: React.FormEvent<HTMLElement>, checked = false) => {
const _choices = [...choices]
_choices[idx].selected = checked
const choice = _choices[idx]
choice.selected = checked
setChoices(_choices)
checked ? defaultSelection.add(choice.c.name) : defaultSelection.delete(choice.c.name)
m.values = [...defaultSelection]
capture(_choices)
},
items = choices.map(({ c, selected }, i) => (
Expand All @@ -104,8 +108,13 @@ export const
styles={{ root: { marginBottom: 4 }, checkmark: { display: 'flex' } }} // Fix: Center the checkmark in the checkbox.
/>
))
React.useEffect(() => { wave.args[m.name] = m.values || [] }, [m.name, m.values])
React.useEffect(() => { setChoices(getMappedChoices()) }, [getMappedChoices, m.choices])

React.useEffect(() => {
const newChoices = getMappedChoices()
setChoices(newChoices)
wave.args[m.name] = newChoices.filter(({ selected }) => selected).map(({ c }) => c.name)
}, [getMappedChoices, m.choices, m.name, m.values])

return (
<div data-test={m.name}>
<Fluent.Label>{m.label}</Fluent.Label>
Expand Down
108 changes: 69 additions & 39 deletions ui/src/combobox.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,21 @@ const
name = 'combobox',
comboboxProps: Combobox = { name, choices: ['A', 'B', 'C'] },
pushMock = jest.fn()
wave.push = pushMock
wave.push = pushMock

describe('Combobox.tsx', () => {
beforeEach(() => pushMock.mockReset())

it('Renders data-test attr', () => {
const { queryByTestId } = render(<XCombobox model={comboboxProps} />)
expect(queryByTestId(name)).toBeInTheDocument()
})

describe('Single Select', () => {
it('Displays new typed option', () => {
it('Sets new option on hitting enter', () => {
const { getByRole } = render(<XCombobox model={{ ...comboboxProps, value: 'A' }} />)
expect(wave.args[name]).toEqual('A')

userEvent.type(getByRole('combobox'), '{backspace}D{enter}')
expect(wave.args[name]).toEqual('D')
})
Expand All @@ -44,16 +46,25 @@ describe('Combobox.tsx', () => {
const combobox = getByRole('combobox')

userEvent.type(combobox, '{backspace}D')
fireEvent.blur(combobox)

// Need to update JSDOM to 16.3+ to use fireEvent.blur().
combobox.blur()
fireEvent.focusOut(combobox)

expect(combobox).toHaveValue('D')
expect(wave.args[name]).toEqual('D')
})

it('Adds new typed option only once to options list', () => {
const { getAllByRole, getAllByTitle, getByRole } = render(<XCombobox model={{ ...comboboxProps, value: 'A' }} />)
const { getAllByRole, getByRole, queryAllByTitle, } = render(<XCombobox model={{ ...comboboxProps, value: 'A' }} />)
fireEvent.click(getByRole('presentation', { hidden: true }))
expect(getAllByRole('option')).toHaveLength(3)
expect(queryAllByTitle('D')).toHaveLength(0)

userEvent.type(getByRole('combobox'), '{backspace}D{enter}')
fireEvent.click(getByRole('presentation', { hidden: true }))
expect(getAllByRole('option')).toHaveLength(4)
expect(getAllByTitle('D')).toHaveLength(1)
expect(queryAllByTitle('D')).toHaveLength(1)
})

describe('Wave args', () => {
Expand All @@ -65,7 +76,7 @@ describe('Combobox.tsx', () => {
render(<XCombobox model={{ ...comboboxProps, value: 'D' }} />)
expect(wave.args[name]).toBe('D')
})

it('Sets args to manually selected option', () => {
const { getByRole, getByTitle } = render(<XCombobox model={{ ...comboboxProps }} />)
fireEvent.click(getByRole('presentation', { hidden: true }))
Expand All @@ -79,10 +90,10 @@ describe('Combobox.tsx', () => {

it('Calls sync when trigger is on', () => {
const { getByRole, getByText } = render(<XCombobox model={{ ...comboboxProps, trigger: true }} />)

fireEvent.click(getByRole('presentation', { hidden: true }))
fireEvent.click(getByText('A'))

expect(pushMock).toHaveBeenCalled()
})

Expand All @@ -95,21 +106,22 @@ describe('Combobox.tsx', () => {

it('Sets wave args as string when a new valued is typed and user clicks away - after init', () => {
const { getByRole } = render(<XCombobox model={{ ...comboboxProps, value: 'A' }} />)

const combobox = getByRole('combobox')

expect(wave.args[name]).toBe('A')

userEvent.type(getByRole('combobox'), '{backspace}D')
// fireEvent.blur(getByRole('combobox')) doesn't trigger blur. Might be related to https://github.com/testing-library/user-event/issues/592
getByRole('combobox').blur()
fireEvent.focusOut(getByRole('combobox'))
// Need to update JSDOM to 16.3+ to use fireEvent.blur().
combobox.blur()
fireEvent.focusOut(combobox)

expect(getByRole('combobox')).not.toHaveFocus()
expect(wave.args[name]).toBe('D')
})

it('Sets wave args as string when a new valued is typed and tab is pressed - after init', () => {
it('Sets wave args as string when a new value is typed and tab is pressed - after init', () => {
const { getByRole } = render(<XCombobox model={{ ...comboboxProps, value: 'A' }} />)

expect(wave.args[name]).toBe('A')

userEvent.type(getByRole('combobox'), '{backspace}D')
Expand All @@ -125,7 +137,7 @@ describe('Combobox.tsx', () => {
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)
})
Expand All @@ -134,47 +146,65 @@ describe('Combobox.tsx', () => {
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('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 all "choices"', () => {
const { getByRole, queryByText, rerender } = render(<XCombobox model={comboboxProps} />)
expect(getByRole('combobox')).not.toHaveValue()

fireEvent.click(getByRole('presentation', { hidden: true }))
expect(queryByText('A')).toBeInTheDocument()
expect(queryByText('B')).toBeInTheDocument()
expect(queryByText('C')).toBeInTheDocument()

rerender(<XCombobox model={{ ...comboboxProps, choices: undefined }} />)
expect(getByRole('combobox')).not.toHaveValue()

fireEvent.click(getByRole('presentation', { hidden: true }))
expect(queryByText('A')).not.toBeInTheDocument()
expect(queryByText('B')).not.toBeInTheDocument()
expect(queryByText('C')).not.toBeInTheDocument()
})

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('Adds initial value to options if it\'s not included in "choices" prop', () => {
const { getByTitle, getAllByRole, getByRole } = render(<XCombobox model={{ ...comboboxProps, value: 'Z' }} />)
expect(wave.args[name]).toEqual('Z')
Expand All @@ -187,7 +217,7 @@ describe('Combobox.tsx', () => {
it('Adds value to choices when both are updated and value was included in previous choices but not in the new choices', () => {
const { getByRole, getAllByRole, getByTitle, rerender } = render(<XCombobox model={{ ...comboboxProps, value: 'A' }} />)
expect(getByRole('combobox')).toHaveValue('A')

rerender(<XCombobox model={{ ...comboboxProps, value: 'C', choices: ['A', 'B'] }} />)
expect(getByRole('combobox')).toHaveValue('C')
fireEvent.click(getByRole('presentation', { hidden: true }))
Expand All @@ -196,7 +226,7 @@ describe('Combobox.tsx', () => {
})

it('Display same value if choices change and value is included in choices', () => {
const { getByRole, rerender, } = render(<XCombobox model={{ ...comboboxProps, value: 'A' }} />, { })
const { getByRole, rerender, } = render(<XCombobox model={{ ...comboboxProps, value: 'A' }} />, {})
expect(getByRole('combobox')).toHaveValue('A')
rerender(<XCombobox model={{ ...comboboxProps, choices: ['A', 'B'], value: 'A' }} />)
expect(getByRole('combobox')).toHaveValue('A')
Expand All @@ -217,27 +247,27 @@ describe('Combobox.tsx', () => {
it('Displays new typed option', () => {
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 { getByText, getByRole } = render(<XCombobox model={{ ...comboboxProps, values: ['A', 'B'] }} />)
expect(wave.args[name]).toEqual(['A', 'B'])

fireEvent.click(getByRole('presentation', { hidden: true }))
fireEvent.click(getByText('A'))
fireEvent.click(getByText('B'))
expect(wave.args[name]).toEqual([])

userEvent.type(getByRole('combobox'), 'D{Enter}')
expect(wave.args[name]).toEqual(['D'])
})

describe('Wave args', () => {
it('Sets args to null when "values" is empty', () => {
render(<XCombobox model={{...comboboxProps, values: []}} />)
render(<XCombobox model={{ ...comboboxProps, values: [] }} />)
expect(wave.args[name]).toBeNull()
})

Expand All @@ -251,17 +281,17 @@ describe('Combobox.tsx', () => {
fireEvent.click(getByRole('presentation', { hidden: true }))
fireEvent.click(getByText('A'))
fireEvent.click(getByText('B'))

expect(wave.args[name]).toEqual(['A', 'B'])
})

it('Calls sync when trigger is on', () => {
const { getByRole, getByText } = render(<XCombobox model={{ ...comboboxProps, values: [], trigger: true }} />)

fireEvent.click(getByRole('presentation', { hidden: true }))
fireEvent.click(getByText('A'))
fireEvent.click(getByText('B'))

expect(pushMock).toHaveBeenCalled()
})
})
Expand All @@ -270,24 +300,24 @@ describe('Combobox.tsx', () => {
it('Types new option and then updates combobox value when "values" prop changes', () => {
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('Displays new options in options list when "choices" prop is updated', () => {
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)
Expand Down
4 changes: 2 additions & 2 deletions ui/src/combobox.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,11 @@ const ComboboxMultiSelect = ({ model: m }: { model: Omit<Combobox, 'value'> }) =
)
}

const useOptions = (choices: S[] = []): [Fluent.IComboBoxOption[], React.Dispatch<React.SetStateAction<Fluent.IComboBoxOption[]>>] => {
const useOptions = (choices?: S[]): [Fluent.IComboBoxOption[], React.Dispatch<React.SetStateAction<Fluent.IComboBoxOption[]>>] => {
const mappedChoices = React.useMemo(() => (choices || []).map((text): Fluent.IComboBoxOption => ({ key: text, text })), [choices])
const [options, setOptions] = React.useState(mappedChoices)

React.useEffect(() => { setOptions(mappedChoices) }, [choices, mappedChoices])
React.useEffect(() => setOptions(mappedChoices), [choices, mappedChoices])

return [options, setOptions]
}
Loading

0 comments on commit 3a252a0

Please sign in to comment.