diff --git a/ui/src/checklist.test.tsx b/ui/src/checklist.test.tsx index e57e1155c22..b957644ade3 100644 --- a/ui/src/checklist.test.tsx +++ b/ui/src/checklist.test.tsx @@ -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', () => { @@ -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() + expect(wave.args[name]).toMatchObject(['Choice1']) + + rerender() + expect(wave.args[name]).toMatchObject([]) + + fireEvent.click(getByText('Choice2').parentElement!) + expect(wave.args[name]).toMatchObject(['Choice2']) + + rerender() + expect(wave.args[name]).toMatchObject([]) + }) + it('Updates choices', () => { const { rerender, getByText } = render() expect(getByText('Choice1')).toBeInTheDocument() diff --git a/ui/src/checklist.tsx b/ui/src/checklist.tsx index 957e29a808b..b16990b90fe 100644 --- a/ui/src/checklist.tsx +++ b/ui/src/checklist.tsx @@ -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, 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) => ( @@ -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 (
{m.label} diff --git a/ui/src/combobox.test.tsx b/ui/src/combobox.test.tsx index 1b12b438ffb..20413c855e3 100644 --- a/ui/src/combobox.test.tsx +++ b/ui/src/combobox.test.tsx @@ -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() expect(queryByTestId(name)).toBeInTheDocument() }) describe('Single Select', () => { - it('Displays new typed option', () => { + it('Sets new option on hitting enter', () => { const { getByRole } = render() expect(wave.args[name]).toEqual('A') - + userEvent.type(getByRole('combobox'), '{backspace}D{enter}') expect(wave.args[name]).toEqual('D') }) @@ -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() + const { getAllByRole, getByRole, queryAllByTitle, } = render() + 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', () => { @@ -65,7 +76,7 @@ describe('Combobox.tsx', () => { render() expect(wave.args[name]).toBe('D') }) - + it('Sets args to manually selected option', () => { const { getByRole, getByTitle } = render() fireEvent.click(getByRole('presentation', { hidden: true })) @@ -79,10 +90,10 @@ describe('Combobox.tsx', () => { it('Calls sync when trigger is on', () => { const { getByRole, getByText } = render() - + fireEvent.click(getByRole('presentation', { hidden: true })) fireEvent.click(getByText('A')) - + expect(pushMock).toHaveBeenCalled() }) @@ -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() - + 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() - + expect(wave.args[name]).toBe('A') userEvent.type(getByRole('combobox'), '{backspace}D') @@ -125,7 +137,7 @@ describe('Combobox.tsx', () => { const { getByRole, getAllByRole, rerender } = render() fireEvent.click(getByRole('presentation', { hidden: true })) expect(getAllByRole('option')).toHaveLength(3) - + rerender() expect(getAllByRole('option')).toHaveLength(2) }) @@ -134,47 +146,65 @@ describe('Combobox.tsx', () => { const { getByRole, getByText, rerender } = render() expect(getByRole('combobox')).toHaveValue('A') expect(wave.args[name]).toEqual('A') - + rerender() 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() 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() expect(wave.args[name]).toEqual('A') expect(getByRole('combobox')).toHaveValue('A') - + rerender() expect(getByRole('combobox')).toHaveValue('B') }) - + + it('Clears all "choices"', () => { + const { getByRole, queryByText, rerender } = render() + expect(getByRole('combobox')).not.toHaveValue() + + fireEvent.click(getByRole('presentation', { hidden: true })) + expect(queryByText('A')).toBeInTheDocument() + expect(queryByText('B')).toBeInTheDocument() + expect(queryByText('C')).toBeInTheDocument() + + rerender() + 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() 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() 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() expect(wave.args[name]).toEqual('Z') @@ -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() expect(getByRole('combobox')).toHaveValue('A') - + rerender() expect(getByRole('combobox')).toHaveValue('C') fireEvent.click(getByRole('presentation', { hidden: true })) @@ -196,7 +226,7 @@ describe('Combobox.tsx', () => { }) it('Display same value if choices change and value is included in choices', () => { - const { getByRole, rerender, } = render(, { }) + const { getByRole, rerender, } = render(, {}) expect(getByRole('combobox')).toHaveValue('A') rerender() expect(getByRole('combobox')).toHaveValue('A') @@ -217,7 +247,7 @@ describe('Combobox.tsx', () => { it('Displays new typed option', () => { const { getByRole } = render() expect(wave.args[name]).toEqual(['A']) - + userEvent.type(getByRole('combobox'), 'D{enter}') expect(wave.args[name]).toEqual(['A', 'D']) }) @@ -225,19 +255,19 @@ describe('Combobox.tsx', () => { it('Unselects every option and types a new one', () => { const { getByText, getByRole } = render() 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() + render() expect(wave.args[name]).toBeNull() }) @@ -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() - + fireEvent.click(getByRole('presentation', { hidden: true })) fireEvent.click(getByText('A')) fireEvent.click(getByText('B')) - + expect(pushMock).toHaveBeenCalled() }) }) @@ -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() expect(getByRole('combobox')).toHaveValue('A, B') - + rerender() 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() expect(getByRole('combobox')).toHaveValue('A, B') }) - + it('Displays new options in options list when "choices" prop is updated', () => { const { getByRole, getAllByRole, rerender } = render() fireEvent.click(getByRole('presentation', { hidden: true })) expect(getAllByRole('option')).toHaveLength(3) - + rerender() fireEvent.click(getByRole('presentation', { hidden: true })) expect(getAllByRole('option')).toHaveLength(2) diff --git a/ui/src/combobox.tsx b/ui/src/combobox.tsx index f1d2c99ff28..e44c3c4717c 100644 --- a/ui/src/combobox.tsx +++ b/ui/src/combobox.tsx @@ -160,11 +160,11 @@ const ComboboxMultiSelect = ({ model: m }: { model: Omit }) = ) } -const useOptions = (choices: S[] = []): [Fluent.IComboBoxOption[], React.Dispatch>] => { +const useOptions = (choices?: S[]): [Fluent.IComboBoxOption[], React.Dispatch>] => { 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] } diff --git a/ui/src/dropdown.test.tsx b/ui/src/dropdown.test.tsx index ff7846f8bde..833f33ad2f5 100644 --- a/ui/src/dropdown.test.tsx +++ b/ui/src/dropdown.test.tsx @@ -730,6 +730,21 @@ describe('Dropdown.tsx', () => { expect(getByText('Choice E')).toBeInTheDocument() expect(getByText('Choice F')).toBeInTheDocument() }) + + + it('Removes all choices of single-valued dialog dropdown', () => { + const + choices = [{ name: 'A', label: 'Choice A' }], + { getByTestId, getByText, queryByText, rerender } = render() + + fireEvent.click(getByTestId(name)) + expect(getByText('Choice A')).toBeInTheDocument() + + rerender() + fireEvent.click(getByTestId(name)) + + expect(queryByText('Choice A')).not.toBeInTheDocument() + }) }) describe('Multi-valued', () => { @@ -848,6 +863,20 @@ describe('Dropdown.tsx', () => { expect(getByText('Choice E')).toBeInTheDocument() expect(getByText('Choice F')).toBeInTheDocument() }) + + it('Removes all choices of multi-valued dialog dropdown', () => { + const + choices = [{ name: 'A', label: 'Choice A' }], + { getByTestId, getByText, queryByText, rerender } = render() + + fireEvent.click(getByTestId(name)) + expect(getByText('Choice A')).toBeInTheDocument() + + rerender() + fireEvent.click(getByTestId(name)) + + expect(queryByText('Choice A')).not.toBeInTheDocument() + }) }) }) }) diff --git a/ui/src/dropdown.tsx b/ui/src/dropdown.tsx index 016ef005150..b3538426abf 100644 --- a/ui/src/dropdown.tsx +++ b/ui/src/dropdown.tsx @@ -168,9 +168,9 @@ const ROW_HEIGHT = 44, PAGE_SIZE = 40, getPageSpecification = () => ({ itemCount: PAGE_SIZE, height: ROW_HEIGHT * PAGE_SIZE } as Fluent.IPageSpecification), - choicesToItems = (choices: Choice[], v?: S | S[]) => choices.map(({ name, label, disabled = false }, idx) => + choicesToItems = (choices: Choice[] = [], v?: S | S[]) => choices.map(({ name, label, disabled = false }, idx) => ({ name, text: label || name, idx, checked: Array.isArray(v) ? v.includes(name) : v === name, show: true, disabled })), - useItems = (choices: Choice[], v?: S | S[]) => { + useItems = (choices?: Choice[], v?: S | S[]) => { const [items, setItems] = React.useState(choicesToItems(choices, v)) const onSearchChange = (_e?: React.ChangeEvent, newVal = '') => setItems(items => items.map(i => ({ ...i, show: fuzzysearch(i.text, newVal) }))) @@ -192,7 +192,7 @@ const DialogDropdownSingle = ({ model }: { model: Dropdown }) => { const - { name, choices = [], disabled, required, trigger, placeholder, label } = model, + { name, choices, disabled, required, trigger, placeholder, label } = model, [isDialogHidden, setIsDialogHidden] = React.useState(true), [items, setItems, onSearchChange] = useItems(choices, model.value), toggleDialog = () => setIsDialogHidden(v => !v), @@ -242,7 +242,7 @@ const }, DialogDropdownMulti = ({ model }: { model: Dropdown }) => { const - { name, choices = [], values = [], disabled, required, trigger, placeholder, label } = model, + { name, choices, values, disabled, required, trigger, placeholder, label } = model, [isDialogHidden, setIsDialogHidden] = React.useState(true), [items, setItems, onSearchChange] = useItems(choices, values), itemsOnDialogOpen = React.useRef(items), @@ -272,7 +272,7 @@ const } React.useEffect(() => { - wave.args[name] = values + wave.args[name] = values || [] setItems(choicesToItems(choices, values)) }, [name, values, choices, setItems]) diff --git a/ui/src/header.tsx b/ui/src/header.tsx index 746ee01dc81..b340ff5412f 100644 --- a/ui/src/header.tsx +++ b/ui/src/header.tsx @@ -107,6 +107,7 @@ const Navigation = bond(({ items, isOpenB }: { items: NavGroup[], isOpenB: Box }) => { const hideNav = () => isOpenB(false), + valueB = box(), render = () => ( - + ) - return { render, isOpenB } + return { render, isOpenB, valueB } }) diff --git a/ui/src/nav.test.tsx b/ui/src/nav.test.tsx index 17375e1294c..99b5c2650cb 100644 --- a/ui/src/nav.test.tsx +++ b/ui/src/nav.test.tsx @@ -61,6 +61,11 @@ describe('Nav.tsx', () => { expect(wave.args[name]).toBeUndefined() }) + it('No item is selected by default', () => { + const { container } = render() + expect(container.querySelector('.is-selected')).not.toBeInTheDocument() + }) + it('Makes link active when value specified', () => { const props: T.Model = { ...navProps, state: { ...navProps.state, value: 'nav1' } } const { getByTitle } = render() @@ -154,9 +159,8 @@ describe('Nav.tsx', () => { it('Selects nav item on value update', () => { const props: T.Model = { ...navProps, state: { items } } - const { rerender, getByTitle } = render() - expect(getByTitle('Nav 1').parentElement).toHaveClass('is-selected') - expect(getByTitle('Nav 2').parentElement).not.toHaveClass('is-selected') + const { container, rerender, getByTitle } = render() + expect(container.querySelector('.is-selected')).not.toBeInTheDocument() props.state.value = 'nav2' rerender() @@ -176,9 +180,10 @@ describe('Nav.tsx', () => { expect(getByTitle('Nav 1').parentElement).not.toHaveClass('is-selected') expect(getByTitle('Nav 2').parentElement).toHaveClass('is-selected') }, - { rerender, getByTitle } = render() + { container, rerender, getByTitle } = render() - expectFirstSelected() + + expect(container.querySelector('.is-selected')).not.toBeInTheDocument() props.state.value = 'nav2' rerender() @@ -195,6 +200,19 @@ describe('Nav.tsx', () => { expectSecondSelected() }) + it('Unselect all nav items', () => { + const props: T.Model = { ...navProps, state: { items, value: 'nav1' } } + const { container, rerender, getByTitle } = render() + + expect(getByTitle('Nav 1').parentElement).toHaveClass('is-selected') + expect(getByTitle('Nav 2').parentElement).not.toHaveClass('is-selected') + + props.state.value = undefined + rerender() + + expect(container.querySelector('.is-selected')).not.toBeInTheDocument() + }) + it('Does not set args on value update when name starts with hash', () => { const props: T.Model = { ...navProps, state: { items: hashItems } } const { rerender } = render() diff --git a/ui/src/nav.tsx b/ui/src/nav.tsx index 3b69ddcc5d3..eb38d033194 100644 --- a/ui/src/nav.tsx +++ b/ui/src/nav.tsx @@ -144,7 +144,7 @@ export const } })) })) - return + return }, View = bond(({ name, state, changed }: Model) => { const @@ -172,8 +172,9 @@ export const update = (prevProps: Model) => { if (prevProps.state.value === valueB()) return valueB(prevProps.state.value) - const name = prevProps.state.value || prevProps.state.items[0].items[0].name + const name = prevProps.state.value + if (!name) return if (name.startsWith('#')) window.location.hash = name.substring(1) else wave.args[name] = true }, diff --git a/ui/src/tab.test.tsx b/ui/src/tab.test.tsx index 1da10d3063c..f9b4ed977a3 100644 --- a/ui/src/tab.test.tsx +++ b/ui/src/tab.test.tsx @@ -21,63 +21,65 @@ import { wave } from './ui' const name = 'tab', hashName = `#${name}`, - tabProps: T.Model = { + items = [{ name: 'tab1' }, { name: 'tab2' }], + getProps = (): T.Model => ({ name, - state: { - items: [{ name }] - }, + state: { items }, changed: T.box(false) - } + }), + pushMock = jest.fn() describe('Tab.tsx', () => { + beforeAll(() => { + wave.push = pushMock + }) beforeEach(() => { window.location.hash = '' + wave.args = [] as any wave.args[name] = null jest.clearAllMocks() }) it('Renders data-test attr', () => { - const { queryByTestId } = render() + const { queryByTestId } = render() expect(queryByTestId(name)).toBeInTheDocument() }) - it('Sets args and calls sync on click', () => { - const pushMock = jest.fn() - wave.push = pushMock - - const { getByRole } = render() - fireEvent.click(getByRole('tab')) + it('Sets args and calls push on click - name is not defined', () => { + const { getAllByRole } = render() + fireEvent.click(getAllByRole('tab')[1]) - expect(wave.args[name]).toBe(true) - expect(pushMock).toHaveBeenCalled() + expect(wave.args['tab2']).toBe(true) + expect(pushMock).toHaveBeenCalledTimes(1) }) - it('Does not set args and calls sync on click - hash name', () => { - const pushMock = jest.fn() - wave.push = pushMock + it('Sets args and calls push on click - name is defined', () => { + const { getAllByRole } = render() + fireEvent.click(getAllByRole('tab')[1]) - const { getByRole } = render() - fireEvent.click(getByRole('tab')) + expect(wave.args[name]).toBe('tab2') + expect(wave.args['tab1']).toBeUndefined() + expect(pushMock).toHaveBeenCalledTimes(1) + }) + + it('Does not call push on click selecting already selected', () => { + const { getAllByRole } = render() + fireEvent.click(getAllByRole('tab')[0]) - expect(wave.args[name]).toBeNull() expect(pushMock).toHaveBeenCalledTimes(0) }) - it('Set args when value is updated', () => { - const items = [{ name: 'tab1' }, { name: 'tab2' }] - const props = { ...tabProps, state: { items, value: 'tab1' } } - const { rerender, getAllByRole } = render() - expect(getAllByRole('tab')[0]).toHaveClass('is-selected') - expect(wave.args['tab2']).toBeUndefined() + it('Does not set args and calls push on click - hash name', () => { + const { getByRole } = render() + fireEvent.click(getByRole('tab')) - props.state.value = 'tab2' - rerender() - expect(wave.args['tab2']).toBe(true) + expect(wave.args[name]).toBeNull() + expect(pushMock).toHaveBeenCalledTimes(0) }) it('Does not set args when value is updated - hash name', () => { const items = [{ name: '#tab1' }, { name: '#tab2' }] - const props = { ...tabProps, state: { items, value: '#tab1' } } + const props = { ...getProps(), state: { items, value: '#tab1' } } const { rerender } = render() expect(wave.args[name]).toBeNull() @@ -85,11 +87,12 @@ describe('Tab.tsx', () => { rerender() expect(wave.args[name]).toBeNull() + expect(pushMock).toHaveBeenCalledTimes(0) }) it('Selects tab when value is updated', () => { const items = [{ name: 'tab1' }, { name: 'tab2' }] - const props = { ...tabProps, state: { items, value: 'tab1' } } + const props = { ...getProps(), state: { items, value: 'tab1' } } const { rerender, getAllByRole } = render() expect(getAllByRole('tab')[0]).toHaveClass('is-selected') expect(getAllByRole('tab')[1]).not.toHaveClass('is-selected') @@ -98,11 +101,12 @@ describe('Tab.tsx', () => { rerender() expect(getAllByRole('tab')[0]).not.toHaveClass('is-selected') expect(getAllByRole('tab')[1]).toHaveClass('is-selected') + expect(pushMock).toHaveBeenCalledTimes(0) }) it('Selects tab when value is updated twice to the same value', () => { const items = [{ name: 'tab1' }, { name: 'tab2' }] - const props = { ...tabProps, state: { items, value: 'tab1' } } + const props = { ...getProps(), state: { items, value: 'tab1' } } const { rerender, getAllByRole } = render() expect(getAllByRole('tab')[0]).toHaveClass('is-selected') expect(getAllByRole('tab')[1]).not.toHaveClass('is-selected') @@ -111,20 +115,24 @@ describe('Tab.tsx', () => { rerender() expect(getAllByRole('tab')[0]).not.toHaveClass('is-selected') expect(getAllByRole('tab')[1]).toHaveClass('is-selected') + expect(pushMock).toHaveBeenCalledTimes(0) fireEvent.click(getAllByRole('tab')[0]) expect(getAllByRole('tab')[0]).toHaveClass('is-selected') expect(getAllByRole('tab')[1]).not.toHaveClass('is-selected') + expect(pushMock).toHaveBeenCalledTimes(1) + pushMock.mockReset() props.state.value = 'tab2' rerender() expect(getAllByRole('tab')[0]).not.toHaveClass('is-selected') expect(getAllByRole('tab')[1]).toHaveClass('is-selected') + expect(pushMock).toHaveBeenCalledTimes(0) }) it('Selects tab when value is updated - hash name', () => { const items = [{ name: '#tab1' }, { name: '#tab2' }] - const props = { ...tabProps, state: { items, value: '#tab1' } } + const props = { ...getProps(), state: { items, value: '#tab1' } } const { rerender, getAllByRole } = render() expect(getAllByRole('tab')[0]).toHaveClass('is-selected') expect(getAllByRole('tab')[1]).not.toHaveClass('is-selected') @@ -133,11 +141,12 @@ describe('Tab.tsx', () => { rerender() expect(getAllByRole('tab')[0]).not.toHaveClass('is-selected') expect(getAllByRole('tab')[1]).toHaveClass('is-selected') + expect(pushMock).toHaveBeenCalledTimes(0) }) it('Selects tab when value is updated twice to the same value - hash name', () => { const items = [{ name: '#tab1' }, { name: '#tab2' }] - const props = { ...tabProps, state: { items, value: '#tab1' } } + const props = { ...getProps(), state: { items, value: '#tab1' } } const { rerender, getAllByRole } = render() expect(getAllByRole('tab')[0]).toHaveClass('is-selected') expect(getAllByRole('tab')[1]).not.toHaveClass('is-selected') @@ -146,40 +155,39 @@ describe('Tab.tsx', () => { rerender() expect(getAllByRole('tab')[0]).not.toHaveClass('is-selected') expect(getAllByRole('tab')[1]).toHaveClass('is-selected') + expect(pushMock).toHaveBeenCalledTimes(0) fireEvent.click(getAllByRole('tab')[0]) expect(getAllByRole('tab')[0]).toHaveClass('is-selected') expect(getAllByRole('tab')[1]).not.toHaveClass('is-selected') + expect(pushMock).toHaveBeenCalledTimes(0) props.state.value = '#tab2' rerender() expect(getAllByRole('tab')[0]).not.toHaveClass('is-selected') expect(getAllByRole('tab')[1]).toHaveClass('is-selected') + expect(pushMock).toHaveBeenCalledTimes(0) }) it('Sets url hash - hash name', () => { - const { getByRole } = render() - fireEvent.click(getByRole('tab')) + const { getAllByRole } = render() + fireEvent.click(getAllByRole('tab')[1]) expect(window.location.hash).toBe(hashName) }) - it('Sets url hash when value is updated - hash name', () => { - const items = [{ name: '#tab1' }, { name: '#tab2' }] - const props = { ...{ ...tabProps, state: { items, value: '#tab1' } } } - const { rerender } = render() - expect(window.location.hash).toBe('') + it('Sets default tab', () => { + const items = [{ name: 'tab1' }, { name: 'tab2' }] + const { getAllByRole } = render() - props.state.value = '#tab2' - rerender() - expect(window.location.hash).toBe('#tab2') + expect(getAllByRole('tab')[1]).toHaveClass('is-selected') }) - it('Sets default tab', () => { + it('Sets default tab - invalid value', () => { const items = [{ name: 'tab1' }, { name: 'tab2' }] - const { getAllByRole } = render() + const { getAllByRole } = render() - expect(getAllByRole('tab')[1]).toHaveClass('is-selected') + expect(getAllByRole('tab')[0]).toHaveClass('is-selected') }) }) \ No newline at end of file diff --git a/ui/src/tab.tsx b/ui/src/tab.tsx index 47542b9ddbc..987c2c51e47 100644 --- a/ui/src/tab.tsx +++ b/ui/src/tab.tsx @@ -45,19 +45,20 @@ const export const View = bond(({ name, state, changed }: Model) => { const - valueB = box(state.value), - setArgs = (name: S) => { - if (name.startsWith('#')) window.location.hash = name.substring(1) - else if (state.name) wave.args[state.name] = name - else wave.args[name] = true - }, + valueB = box(state.value || state.items[0]?.name), onLinkClick = (item?: PivotItem) => { const name = item?.props.itemKey - if (!name) return + if (!name || valueB() === name) return state.value = name valueB(name) - setArgs(name) - if (!name.startsWith('#')) wave.push() + + if (name.startsWith('#')) { + window.location.hash = name.substring(1) + return + } + if (state.name) wave.args[state.name] = name + else wave.args[name] = true + wave.push() }, render = () => { const @@ -67,14 +68,12 @@ export const )) return (
- {items} + {items}
) }, update = (prevProps: Model) => { - if (prevProps.state.value === valueB()) return - valueB(prevProps.state.value) - setArgs(prevProps.state.value || prevProps.state.items[0].name) + if (prevProps.state.value !== valueB()) valueB(prevProps.state.value) } return { render, changed, update, valueB } diff --git a/ui/src/tabs.test.tsx b/ui/src/tabs.test.tsx index 1711f0bc40c..87f9ee39cec 100644 --- a/ui/src/tabs.test.tsx +++ b/ui/src/tabs.test.tsx @@ -18,35 +18,162 @@ import { Tabs, XTabs } from './tabs' import { wave } from './ui' const name = 'tabs' -const tabsProps: Tabs = { name, items: [{ name }] } +const hashName = `#${name}` +const getProps = (): Tabs => ({ name, items: [{ name: 'tab1' }, { name: 'tab2' }] }) +const pushMock = jest.fn() describe('Tabs.tsx', () => { - beforeEach(() => { wave.args[name] = null }) + beforeAll(() => { + wave.push = pushMock + }) + beforeEach(() => { + wave.args = [] as any + wave.args[name] = null + pushMock.mockReset() + }) it('Renders data-test attr', () => { - const { queryByTestId } = render() + const { queryByTestId } = render() expect(queryByTestId(name)).toBeInTheDocument() }) - it('Sets args and calls sync on click', () => { - const pushMock = jest.fn() - wave.push = pushMock + it('Sets args and calls push on click - name is an empty string', () => { + const { getAllByRole } = render() + fireEvent.click(getAllByRole('tab')[1]) - const { getByRole } = render() - fireEvent.click(getByRole('tab')) + expect(wave.args['tab2']).toBe(true) + expect(pushMock).toHaveBeenCalledTimes(1) + }) + + it('Sets args and calls push on click', () => { + const { getAllByRole } = render() + fireEvent.click(getAllByRole('tab')[1]) - expect(wave.args[name]).toBe(name) - expect(pushMock).toHaveBeenCalled() + expect(wave.args[name]).toBe('tab2') + expect(wave.args['tab1']).toBeUndefined() + expect(pushMock).toHaveBeenCalledTimes(1) }) - it('Does not call sync on click - args not changed', () => { - const pushMock = jest.fn() - wave.push = pushMock - wave.args[name] = name - const { getByRole } = render() - fireEvent.click(getByRole('tab')) + it('Does not call push on click selecting already selected', () => { + const { getAllByRole } = render() + fireEvent.click(getAllByRole('tab')[0]) expect(pushMock).toHaveBeenCalledTimes(0) }) + it('Does not set args when value is updated - hash name', () => { + const items = [{ name: '#tab1' }, { name: '#tab2' }] + const props = { ...getProps(), items, value: '#tab1' } + const { rerender } = render() + expect(wave.args[name]).toBeNull() + + props.value = '#tab2' + rerender() + + expect(wave.args[name]).toBeNull() + expect(pushMock).toHaveBeenCalledTimes(0) + }) + + it('Selects tab when value is updated', () => { + const items = [{ name: 'tab1' }, { name: 'tab2' }] + const props = { ...getProps(), items, value: 'tab1' } + const { rerender, getAllByRole } = render() + expect(getAllByRole('tab')[0]).toHaveClass('is-selected') + expect(getAllByRole('tab')[1]).not.toHaveClass('is-selected') + expect(pushMock).toHaveBeenCalledTimes(0) + + props.value = 'tab2' + rerender() + expect(getAllByRole('tab')[0]).not.toHaveClass('is-selected') + expect(getAllByRole('tab')[1]).toHaveClass('is-selected') + expect(pushMock).toHaveBeenCalledTimes(0) + }) + + it('Selects tab when value is updated twice to the same value', () => { + const items = [{ name: 'tab1' }, { name: 'tab2' }] + const props = { ...getProps(), items, value: 'tab1' } + const { rerender, getAllByRole } = render() + expect(getAllByRole('tab')[0]).toHaveClass('is-selected') + expect(getAllByRole('tab')[1]).not.toHaveClass('is-selected') + expect(pushMock).toHaveBeenCalledTimes(0) + + props.value = 'tab2' + rerender() + expect(getAllByRole('tab')[0]).not.toHaveClass('is-selected') + expect(getAllByRole('tab')[1]).toHaveClass('is-selected') + expect(pushMock).toHaveBeenCalledTimes(0) + + fireEvent.click(getAllByRole('tab')[0]) + expect(getAllByRole('tab')[0]).toHaveClass('is-selected') + expect(getAllByRole('tab')[1]).not.toHaveClass('is-selected') + expect(pushMock).toHaveBeenCalledTimes(1) + pushMock.mockReset() + + props.value = 'tab2' + rerender() + expect(getAllByRole('tab')[0]).not.toHaveClass('is-selected') + expect(getAllByRole('tab')[1]).toHaveClass('is-selected') + expect(pushMock).toHaveBeenCalledTimes(0) + }) + + it('Selects tab when value is updated - hash name', () => { + const items = [{ name: '#tab1' }, { name: '#tab2' }] + const props = { ...getProps(), items, value: '#tab1' } + const { rerender, getAllByRole } = render() + expect(getAllByRole('tab')[0]).toHaveClass('is-selected') + expect(getAllByRole('tab')[1]).not.toHaveClass('is-selected') + expect(pushMock).toHaveBeenCalledTimes(0) + + props.value = '#tab2' + rerender() + expect(getAllByRole('tab')[0]).not.toHaveClass('is-selected') + expect(getAllByRole('tab')[1]).toHaveClass('is-selected') + expect(pushMock).toHaveBeenCalledTimes(0) + }) + + it('Selects tab when value is updated twice to the same value - hash name', () => { + const items = [{ name: '#tab1' }, { name: '#tab2' }] + const props = { ...getProps(), items, value: '#tab1' } + const { rerender, getAllByRole } = render() + expect(getAllByRole('tab')[0]).toHaveClass('is-selected') + expect(getAllByRole('tab')[1]).not.toHaveClass('is-selected') + + props.value = '#tab2' + rerender() + expect(getAllByRole('tab')[0]).not.toHaveClass('is-selected') + expect(getAllByRole('tab')[1]).toHaveClass('is-selected') + expect(pushMock).toHaveBeenCalledTimes(0) + + fireEvent.click(getAllByRole('tab')[0]) + expect(getAllByRole('tab')[0]).toHaveClass('is-selected') + expect(getAllByRole('tab')[1]).not.toHaveClass('is-selected') + + props.value = '#tab2' + rerender() + expect(getAllByRole('tab')[0]).not.toHaveClass('is-selected') + expect(getAllByRole('tab')[1]).toHaveClass('is-selected') + expect(pushMock).toHaveBeenCalledTimes(0) + }) + + it('Sets url hash - hash name', () => { + const { getByRole } = render() + fireEvent.click(getByRole('tab')) + + expect(window.location.hash).toBe(hashName) + }) + + it('Sets default tab', () => { + const items = [{ name: 'tab1' }, { name: 'tab2' }] + const { getAllByRole } = render() + + expect(getAllByRole('tab')[1]).toHaveClass('is-selected') + }) + + it('Selects first tab if value not defined', () => { + const items = [{ name: 'tab1' }, { name: 'tab2' }] + const { getAllByRole } = render() + + expect(getAllByRole('tab')[0]).toHaveClass('is-selected') + }) + }) \ No newline at end of file diff --git a/ui/src/tabs.tsx b/ui/src/tabs.tsx index fc6e40c4536..5b87c7b1805 100644 --- a/ui/src/tabs.tsx +++ b/ui/src/tabs.tsx @@ -17,6 +17,7 @@ import { B, Id, S } from 'h2o-wave' import React from 'react' import { stylesheet } from 'typestyle' import { wave } from './ui' +import useUpdateOnlyEffect from './parts/useUpdateOnlyEffectHook' /** * Create a tab. @@ -67,12 +68,13 @@ export const const name = item?.props.itemKey if (!name) return setSelected(name) + m.value = name if (name.startsWith('#')) { window.location.hash = name.substring(1) return } if (m.name) { - if (name !== wave.args[m.name]) { + if (name !== selected) { wave.args[m.name] = name wave.push() } @@ -82,9 +84,9 @@ export const } }, tabs = m.items?.map(t => ), - [selected, setSelected] = React.useState(m.value) + [selected, setSelected] = React.useState(m.value || m.items?.[0].name) - React.useEffect(() => setSelected(m.value), [m.value]) + useUpdateOnlyEffect(() => setSelected(m.value), [m.value]) return (