From d93050fe61fd88ffa1b2a8642a0de4ab4891d8b2 Mon Sep 17 00:00:00 2001 From: kamre Date: Sun, 30 Oct 2022 21:07:41 +0700 Subject: [PATCH 1/2] Minor fixes for multi-select property: - width of delete icon set to 16px so that value text is not truncated - don't close menu for multi-select when value is selected or unselected - support `Escape` and `Backspace` keys while editing - unit tests updated and two more added --- .../multiselect/multiselect.test.tsx | 60 +++++++++++++++++++ webapp/src/widgets/valueSelector.scss | 2 +- webapp/src/widgets/valueSelector.tsx | 17 +++++- 3 files changed, 75 insertions(+), 4 deletions(-) diff --git a/webapp/src/properties/multiselect/multiselect.test.tsx b/webapp/src/properties/multiselect/multiselect.test.tsx index 3807d5e40..24d9a3b42 100644 --- a/webapp/src/properties/multiselect/multiselect.test.tsx +++ b/webapp/src/properties/multiselect/multiselect.test.tsx @@ -57,6 +57,12 @@ describe('properties/multiSelect', () => { const board = createBoard() const card = createCard() + const expectOptionsMenuToBeVisible = (template: IPropertyTemplate) => { + for (const option of template.options) { + expect(screen.getByRole('menuitem', {name: option.value})).toBeInTheDocument() + } + } + beforeEach(() => { jest.resetAllMocks() }) @@ -128,6 +134,7 @@ describe('properties/multiSelect', () => { userEvent.type(screen.getByRole('combobox', {name: /value selector/i}), 'b{enter}') expect(mockedMutator.changePropertyValue).toHaveBeenCalledWith(board.id, card, propertyTemplate.id, ['multi-option-1', 'multi-option-2']) + expectOptionsMenuToBeVisible(propertyTemplate) }) it('can unselect a option', async () => { @@ -152,6 +159,58 @@ describe('properties/multiSelect', () => { userEvent.click(screen.getAllByRole('button', {name: /clear/i})[0]) expect(mockedMutator.changePropertyValue).toHaveBeenCalledWith(board.id, card, propertyTemplate.id, ['multi-option-2']) + expectOptionsMenuToBeVisible(propertyTemplate) + }) + + it('can unselect a option via backspace', async () => { + const propertyTemplate = buildMultiSelectPropertyTemplate() + const propertyValue = ['multi-option-1', 'multi-option-2'] + + render( + , + {wrapper: Wrapper}, + ) + + userEvent.click(screen.getByTestId(nonEditableMultiSelectTestId)) + + userEvent.type(screen.getByRole('combobox', {name: /value selector/i}), '{backspace}') + + expect(mockedMutator.changePropertyValue).toHaveBeenCalledWith(board.id, card, propertyTemplate.id, ['multi-option-1']) + expectOptionsMenuToBeVisible(propertyTemplate) + }) + + it('can close menu on escape', async () => { + const propertyTemplate = buildMultiSelectPropertyTemplate() + const propertyValue = ['multi-option-1', 'multi-option-2'] + + render( + , + {wrapper: Wrapper}, + ) + + userEvent.click(screen.getByTestId(nonEditableMultiSelectTestId)) + + userEvent.type(screen.getByRole('combobox', {name: /value selector/i}), '{escape}') + + for (const option of propertyTemplate.options) { + expect(screen.queryByRole('menuitem', {name: option.value})).toBeNull() + } }) it('can create a new option', async () => { @@ -177,6 +236,7 @@ describe('properties/multiSelect', () => { userEvent.type(screen.getByRole('combobox', {name: /value selector/i}), 'new-value{enter}') expect(mockedMutator.insertPropertyOption).toHaveBeenCalledWith(board.id, board.cardProperties, propertyTemplate, expect.objectContaining({value: 'new-value'}), 'add property option') + expectOptionsMenuToBeVisible(propertyTemplate) }) it('can delete a option', () => { diff --git a/webapp/src/widgets/valueSelector.scss b/webapp/src/widgets/valueSelector.scss index 4787dcc40..f64b0202f 100644 --- a/webapp/src/widgets/valueSelector.scss +++ b/webapp/src/widgets/valueSelector.scss @@ -25,8 +25,8 @@ .IconButton.delete-value { @include z-index(value-selector-delete); width: 16px; - min-width: 16px; height: 16px; + flex: 0 0 auto; i { font-size: 16px; diff --git a/webapp/src/widgets/valueSelector.tsx b/webapp/src/widgets/valueSelector.tsx index 155e5273e..4c701c051 100644 --- a/webapp/src/widgets/valueSelector.tsx +++ b/webapp/src/widgets/valueSelector.tsx @@ -71,7 +71,10 @@ const ValueSelectorLabel = (props: LabelProps): JSX.Element => { ) } return ( -
+
@@ -180,24 +183,32 @@ function ValueSelector(props: Props): JSX.Element { getOptionLabel={(o: IPropertyOption) => o.value} getOptionValue={(o: IPropertyOption) => o.id} onChange={(value: OnChangeValue, action: ActionMeta): void => { - if (action.action === 'select-option') { + if (action.action === 'select-option' || action.action === 'pop-value') { if (Array.isArray(value)) { props.onChange((value as IPropertyOption[]).map((option) => option.id)) } else { props.onChange((value as IPropertyOption).id) + props.onBlur?.() } } else if (action.action === 'clear') { props.onChange('') } }} + onKeyDown={(event) => { + if (event.key === 'Escape') { + props.onBlur?.() + } + }} onBlur={props.onBlur} onCreateOption={props.onCreate} autoFocus={true} value={props.value || null} - closeMenuOnSelect={true} + closeMenuOnSelect={!props.isMulti} placeholder={props.emptyValue} hideSelectedOptions={false} defaultMenuIsOpen={true} + menuIsOpen={props.isMulti} + blurInputOnSelect={!props.isMulti} /> ) } From 951a8eceab99a0ad7cfa3b9b92dfef620c90a1ba Mon Sep 17 00:00:00 2001 From: kamre Date: Tue, 22 Nov 2022 10:10:17 +0700 Subject: [PATCH 2/2] Fixed ValueSelector after merge with main. --- webapp/src/widgets/valueSelector.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/webapp/src/widgets/valueSelector.tsx b/webapp/src/widgets/valueSelector.tsx index e77dfcb84..4c701c051 100644 --- a/webapp/src/widgets/valueSelector.tsx +++ b/webapp/src/widgets/valueSelector.tsx @@ -165,7 +165,6 @@ function ValueSelector(props: Props): JSX.Element { captureMenuScroll={true} maxMenuHeight={1200} isMulti={props.isMulti} - menuIsOpen={true} isClearable={true} styles={valueSelectorStyle} formatOptionLabel={(option: IPropertyOption, meta: FormatOptionLabelMeta) => (