[GH-2965] Don't close property selector menu when item is clicked (#3587)

* Don't close menus for board properties or filter values when switch item is toggled.

* Stop click propagation for menu items.

* Jest tests fixed and snapshots updated.

* Fix Cypress tests.

* Unit tests for `FilterValue` and `ViewHeaderPropertiesMenu` checking that menu is not closed added.

* Reverted changes to the dom structure of `MenuWrapper`

webapp/cypress/integration/cardBadges.ts

@@ -54,8 +54,8 @@ describe('Card badges', () => {
 
         // Hide card badges
         cy.log('**Hide card badges**')
-        cy.findByRole('button', {name: 'Properties menu'}).click()
         cy.findByRole('button', {name: 'Comments and description'}).click()
+        cy.findByRole('button', {name: 'Properties menu'}).click()
         cy.findByTitle('This card has a description').should('not.exist')
         cy.findByTitle('Comments').should('not.exist')
         cy.findByTitle('Checkboxes').should('not.exist')

webapp/src/components/cardDetail/__snapshots__/cardDetailContentsMenu.test.tsx.snap

@@ -376,6 +376,162 @@ exports[`components/cardDetail/cardDetailContentsMenu return cardDetailContentsM
           Add content
         </span>
       </button>
+      <div
+        class="Menu noselect top "
+      >
+        <div
+          class="menu-contents"
+        >
+          <div
+            class="menu-options"
+          >
+            <div>
+              <div
+                aria-label="text"
+                class="MenuOption TextOption menu-option"
+                role="button"
+              >
+                <div
+                  class="d-flex"
+                >
+                  <svg
+                    class="TextIcon Icon"
+                    viewBox="0 0 448 512"
+                    xmlns="http://www.w3.org/2000/svg"
+                  >
+                    <path
+                      d="M432 416H16a16 16 0 0 0-16 16v32a16 16 0 0 0 16 16h416a16 16 0 0 0 16-16v-32a16 16 0 0 0-16-16zm0-128H16a16 16 0 0 0-16 16v32a16 16 0 0 0 16 16h416a16 16 0 0 0 16-16v-32a16 16 0 0 0-16-16zm0-128H16a16 16 0 0 0-16 16v32a16 16 0 0 0 16 16h416a16 16 0 0 0 16-16v-32a16 16 0 0 0-16-16zm0-128H16A16 16 0 0 0 0 48v32a16 16 0 0 0 16 16h416a16 16 0 0 0 16-16V48a16 16 0 0 0-16-16z"
+                    />
+                  </svg>
+                </div>
+                <div
+                  class="menu-name"
+                >
+                  text
+                </div>
+                <div
+                  class="noicon"
+                />
+              </div>
+            </div>
+            <div>
+              <div
+                aria-label="image"
+                class="MenuOption TextOption menu-option"
+                role="button"
+              >
+                <div
+                  class="d-flex"
+                >
+                  <svg
+                    class="ImageIcon Icon"
+                    viewBox="0 0 512 512"
+                    xmlns="http://www.w3.org/2000/svg"
+                  >
+                    <path
+                      d="M464 64H48C21.49 64 0 85.49 0 112v288c0 26.51 21.49 48 48 48h416c26.51 0 48-21.49 48-48V112c0-26.51-21.49-48-48-48zm-6 336H54a6 6 0 0 1-6-6V118a6 6 0 0 1 6-6h404a6 6 0 0 1 6 6v276a6 6 0 0 1-6 6zM128 152c-22.091 0-40 17.909-40 40s17.909 40 40 40 40-17.909 40-40-17.909-40-40-40zM96 352h320v-80l-87.515-87.515c-4.686-4.686-12.284-4.686-16.971 0L192 304l-39.515-39.515c-4.686-4.686-12.284-4.686-16.971 0L96 304v48z"
+                    />
+                  </svg>
+                </div>
+                <div
+                  class="menu-name"
+                >
+                  image
+                </div>
+                <div
+                  class="noicon"
+                />
+              </div>
+            </div>
+            <div>
+              <div
+                aria-label="divider"
+                class="MenuOption TextOption menu-option"
+                role="button"
+              >
+                <div
+                  class="d-flex"
+                >
+                  <svg
+                    class="DividerIcon Icon"
+                    viewBox="0 0 448 512"
+                    xmlns="http://www.w3.org/2000/svg"
+                  >
+                    <path
+                      d="M 432,224 H 16 c -8.836556,0 -16,7.16344 -16,16 v 32 c 0,8.83656 7.163444,16 16,16 h 416 c 8.83656,0 16,-7.16344 16,-16 v -32 c 0,-8.83656 -7.16344,-16 -16,-16 z"
+                    />
+                  </svg>
+                </div>
+                <div
+                  class="menu-name"
+                >
+                  divider
+                </div>
+                <div
+                  class="noicon"
+                />
+              </div>
+            </div>
+            <div>
+              <div
+                aria-label="checkbox"
+                class="MenuOption TextOption menu-option"
+                role="button"
+              >
+                <div
+                  class="d-flex"
+                >
+                  <svg
+                    class="CheckIcon Icon"
+                    viewBox="0 0 100 100"
+                    xmlns="http://www.w3.org/2000/svg"
+                  >
+                    <polyline
+                      points="20,60 40,80 80,40"
+                    />
+                  </svg>
+                </div>
+                <div
+                  class="menu-name"
+                >
+                  checkbox
+                </div>
+                <div
+                  class="noicon"
+                />
+              </div>
+            </div>
+          </div>
+          <div
+            class="menu-spacer hideOnWidescreen"
+          />
+          <div
+            class="menu-options hideOnWidescreen"
+          >
+            <div
+              aria-label="Cancel"
+              class="MenuOption TextOption menu-option menu-cancel"
+              role="button"
+            >
+              <div
+                class="d-flex"
+              >
+                <div
+                  class="noicon"
+                />
+              </div>
+              <div
+                class="menu-name"
+              >
+                Cancel
+              </div>
+              <div
+                class="noicon"
+              />
+            </div>
+          </div>
+        </div>
+      </div>
     </div>
   </div>
 </div>

webapp/src/components/viewHeader/__snapshots__/filterValue.test.tsx.snap

@@ -97,6 +97,69 @@ exports[`components/viewHeader/filterValue return filterValue and click Status 1
         Status
       </span>
     </button>
+    <div
+      class="Menu noselect bottom "
+    >
+      <div
+        class="menu-contents"
+      >
+        <div
+          class="menu-options"
+        >
+          <div>
+            <div
+              aria-label="Status"
+              class="MenuOption SwitchOption menu-option"
+              role="button"
+            >
+              <div
+                class="noicon"
+              />
+              <div
+                class="menu-name"
+              >
+                Status
+              </div>
+              <div
+                class="Switch override-main size--small on"
+              >
+                <div
+                  class="octo-switch-inner"
+                />
+              </div>
+            </div>
+          </div>
+        </div>
+        <div
+          class="menu-spacer hideOnWidescreen"
+        />
+        <div
+          class="menu-options hideOnWidescreen"
+        >
+          <div
+            aria-label="Cancel"
+            class="MenuOption TextOption menu-option menu-cancel"
+            role="button"
+          >
+            <div
+              class="d-flex"
+            >
+              <div
+                class="noicon"
+              />
+            </div>
+            <div
+              class="menu-name"
+            >
+              Cancel
+            </div>
+            <div
+              class="noicon"
+            />
+          </div>
+        </div>
+      </div>
+    </div>
   </div>
 </div>
 `;
@@ -116,6 +179,69 @@ exports[`components/viewHeader/filterValue return filterValue and click Status w
         (Unknown)
       </span>
     </button>
+    <div
+      class="Menu noselect bottom "
+    >
+      <div
+        class="menu-contents"
+      >
+        <div
+          class="menu-options"
+        >
+          <div>
+            <div
+              aria-label="Status"
+              class="MenuOption SwitchOption menu-option"
+              role="button"
+            >
+              <div
+                class="noicon"
+              />
+              <div
+                class="menu-name"
+              >
+                Status
+              </div>
+              <div
+                class="Switch override-main size--small"
+              >
+                <div
+                  class="octo-switch-inner"
+                />
+              </div>
+            </div>
+          </div>
+        </div>
+        <div
+          class="menu-spacer hideOnWidescreen"
+        />
+        <div
+          class="menu-options hideOnWidescreen"
+        >
+          <div
+            aria-label="Cancel"
+            class="MenuOption TextOption menu-option menu-cancel"
+            role="button"
+          >
+            <div
+              class="d-flex"
+            >
+              <div
+                class="noicon"
+              />
+            </div>
+            <div
+              class="menu-name"
+            >
+              Cancel
+            </div>
+            <div
+              class="noicon"
+            />
+          </div>
+        </div>
+      </div>
+    </div>
   </div>
 </div>
 `;

webapp/src/components/viewHeader/filterComponent.tsx

@@ -72,7 +72,7 @@ const FilterComponent = (props: Props): JSX.Element => {
             >
                 {filters.map((filter) => (
                     <FilterEntry
-                        key={`${filter.propertyId}-${filter.condition}-${filter.values.join(',')}`}
+                        key={`${filter.propertyId}-${filter.condition}`}
                         board={board}
                         view={activeView}
                         conditionClicked={conditionClicked}

webapp/src/components/viewHeader/filterEntry.tsx

@@ -31,7 +31,7 @@ const FilterEntry = (props: Props): JSX.Element => {
 
     const template = board.cardProperties.find((o: IPropertyTemplate) => o.id === filter.propertyId)
     const propertyName = template ? template.name : '(unknown)'
-    const key = `${filter.propertyId}-${filter.condition}-${filter.values.join(',')}`
+    const key = `${filter.propertyId}-${filter.condition}}`
     return (
         <div
             className='FilterEntry'

webapp/src/components/viewHeader/filterValue.test.tsx

@@ -102,4 +102,25 @@ describe('components/viewHeader/filterValue', () => {
         expect(mockedMutator.changeViewFilter).toBeCalledTimes(1)
         expect(container).toMatchSnapshot()
     })
+    test('return filterValue and verify that menu is not closed after clicking on the item', () => {
+        filter.values = []
+        activeView.fields.filter.filters = [filter]
+        render(
+            wrapIntl(
+                <ReduxProvider store={store}>
+                    <FilterValue
+                        view={activeView}
+                        filter={filter}
+                        template={board.cardProperties[0]}
+                    />
+                </ReduxProvider>,
+            ),
+        )
+        const buttonElement = screen.getByRole('button', {name: '(empty)'})
+        userEvent.click(buttonElement)
+
+        const switchStatus = screen.getByRole('button', {name: 'Status'})
+        userEvent.click(switchStatus)
+        expect(switchStatus).toBeInTheDocument()
+    })
 })

webapp/src/components/viewHeader/filterValue.tsx

@@ -46,6 +46,7 @@ const filterValue = (props: Props): JSX.Element|null => {
                         id={o.id}
                         name={o.value}
                         isOn={filter.values.includes(o.id)}
+                        suppressItemClicked={true}
                         onClick={(optionId) => {
                             const filterIndex = view.fields.filter.filters.indexOf(filter)
                             Utils.assert(filterIndex >= 0, "Can't find filter")

webapp/src/components/viewHeader/viewHeaderPropertiesMenu.test.tsx

@@ -93,4 +93,28 @@ describe('components/viewHeader/viewHeaderPropertiesMenu', () => {
             [...activeView.fields.visiblePropertyIds, Constants.badgesColumnId],
         )
     })
+    test('show menu and verify that it is not closed after clicking on the item', () => {
+        render(
+            wrapIntl(
+                <ReduxProvider store={store}>
+                    <ViewHeaderPropertiesMenu
+                        activeView={activeView}
+                        properties={board.cardProperties}
+                    />
+                </ReduxProvider>,
+            ),
+        )
+        const menuButton = screen.getByRole('button', {name: 'Properties menu'})
+        userEvent.click(menuButton)
+
+        const property1Button = screen.getByRole('button', {name: 'Property 1'})
+        userEvent.click(property1Button)
+        expect(property1Button).toBeInTheDocument()
+
+        const property2Button = screen.getByRole('button', {name: 'Property 2'})
+        userEvent.click(property2Button)
+        expect(property2Button).toBeInTheDocument()
+
+        expect(mockedMutator.changeViewVisibleProperties).toHaveBeenCalledTimes(2)
+    })
 })

webapp/src/components/viewHeader/viewHeaderPropertiesMenu.tsx

@@ -46,6 +46,7 @@ const ViewHeaderPropertiesMenu = (props: Props) => {
                         id={Constants.titleColumnId}
                         name={intl.formatMessage({id: 'default-properties.title', defaultMessage: 'Title'})}
                         isOn={visiblePropertyIds.includes(Constants.titleColumnId)}
+                        suppressItemClicked={true}
                         onClick={toggleVisibility}
                     />}
                 {properties?.map((option: IPropertyTemplate) => (
@@ -54,6 +55,7 @@ const ViewHeaderPropertiesMenu = (props: Props) => {
                         id={option.id}
                         name={option.name}
                         isOn={visiblePropertyIds.includes(option.id)}
+                        suppressItemClicked={true}
                         onClick={toggleVisibility}
                     />
                 ))}
@@ -63,6 +65,7 @@ const ViewHeaderPropertiesMenu = (props: Props) => {
                         id={Constants.badgesColumnId}
                         name={intl.formatMessage({id: 'default-properties.badges', defaultMessage: 'Comments and description'})}
                         isOn={visiblePropertyIds.includes(Constants.badgesColumnId)}
+                        suppressItemClicked={true}
                         onClick={toggleVisibility}
                     />}
             </Menu>

webapp/src/widgets/menu/colorOption.tsx

@@ -22,6 +22,7 @@ function ColorOption(props: ColorOptionProps): JSX.Element {
             onClick={(e: React.MouseEvent): void => {
                 e.target.dispatchEvent(new Event('menuItemClicked'))
                 props.onClick(props.id)
+                e.stopPropagation()
             }}
         >
             {icon ?? <div className='noicon'/>}

webapp/src/widgets/menu/switchOption.tsx

@@ -9,10 +9,11 @@ import {MenuOptionProps} from './menuItem'
 type SwitchOptionProps = MenuOptionProps & {
     isOn: boolean,
     icon?: React.ReactNode,
+    suppressItemClicked?: boolean
 }
 
 function SwitchOption(props: SwitchOptionProps): JSX.Element {
-    const {name, icon, isOn} = props
+    const {name, icon, isOn, suppressItemClicked} = props
 
     return (
         <div
@@ -20,8 +21,10 @@ function SwitchOption(props: SwitchOptionProps): JSX.Element {
             role='button'
             aria-label={name}
             onClick={(e: React.MouseEvent) => {
-                e.target.dispatchEvent(new Event('menuItemClicked'))
+                if (!suppressItemClicked)
+                    e.target.dispatchEvent(new Event('menuItemClicked'))
                 props.onClick(props.id)
+                e.stopPropagation()
             }}
         >
             {icon ?? <div className='noicon'/>}

webapp/src/widgets/menu/textOption.tsx

@@ -25,6 +25,7 @@ function TextOption(props:TextOptionProps): JSX.Element {
             onClick={(e: React.MouseEvent) => {
                 e.target.dispatchEvent(new Event('menuItemClicked'))
                 props.onClick(props.id)
+                e.stopPropagation()
             }}
         >
             <div className={`${check ? 'd-flex menu-option__check' : 'd-flex'}`}>{icon ?? <div className='noicon'/>}</div>

webapp/src/widgets/menuWrapper.tsx

@@ -68,15 +68,19 @@ const MenuWrapper = (props: Props) => {
     }, [props.onToggle, open, props.disabled])
 
     useEffect(() => {
-        document.addEventListener('menuItemClicked', close, true)
-        document.addEventListener('click', closeOnBlur, true)
-        document.addEventListener('keyup', keyboardClose, true)
-        return () => {
-            document.removeEventListener('menuItemClicked', close, true)
-            document.removeEventListener('click', closeOnBlur, true)
-            document.removeEventListener('keyup', keyboardClose, true)
+        if (open) {
+            document.addEventListener('menuItemClicked', close, true)
+            document.addEventListener('click', closeOnBlur, true)
+            document.addEventListener('keyup', keyboardClose, true)
         }
-    }, [close, closeOnBlur, keyboardClose])
+        return () => {
+            if (open) {
+                document.removeEventListener('menuItemClicked', close, true)
+                document.removeEventListener('click', closeOnBlur, true)
+                document.removeEventListener('keyup', keyboardClose, true)
+            }
+        }
+    }, [open, close, closeOnBlur, keyboardClose])
 
     const {children} = props
     let className = 'MenuWrapper'