From d93050fe61fd88ffa1b2a8642a0de4ab4891d8b2 Mon Sep 17 00:00:00 2001
From: kamre <eremchenko@gmail.com>
Date: Sun, 30 Oct 2022 21:07:41 +0700
Subject: [PATCH] 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(
+            <MultiSelect
+                property={new MultiSelectProperty()}
+                readOnly={false}
+                showEmptyPlaceholder={false}
+                propertyTemplate={propertyTemplate}
+                propertyValue={propertyValue}
+                board={{...board}}
+                card={{...card}}
+            />,
+            {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(
+            <MultiSelect
+                property={new MultiSelectProperty()}
+                readOnly={false}
+                showEmptyPlaceholder={false}
+                propertyTemplate={propertyTemplate}
+                propertyValue={propertyValue}
+                board={{...board}}
+                card={{...card}}
+            />,
+            {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 (
-        <div className='value-menu-option'>
+        <div
+            className='value-menu-option'
+            role='menuitem'
+        >
             <div className='label-container'>
                 <Label color={option.color}>{option.value}</Label>
             </div>
@@ -180,24 +183,32 @@ function ValueSelector(props: Props): JSX.Element {
             getOptionLabel={(o: IPropertyOption) => o.value}
             getOptionValue={(o: IPropertyOption) => o.id}
             onChange={(value: OnChangeValue<IPropertyOption, true | false>, action: ActionMeta<IPropertyOption>): 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}
         />
     )
 }