Browse Source

Maintenance: Desktop-View - Improved handling of 2FA configuration and action menu output without actions.

Dominik Klein 10 months ago
parent
commit
2ef16a35cf

+ 34 - 31
app/frontend/apps/desktop/components/CommonActionMenu/CommonActionMenu.vue

@@ -18,7 +18,7 @@ import type {
 } from '#desktop/components/CommonPopover/types.ts'
 import { usePopoverMenu } from '#desktop/components/CommonPopover/usePopoverMenu.ts'
 
-interface Props {
+export interface Props {
   actions: MenuItem[]
   entity?: ObjectLike
   buttonSize?: ButtonSize
@@ -67,7 +67,10 @@ const buttonVariantClass = computed(() => {
 </script>
 
 <template>
-  <div v-if="filteredMenuItems" class="inline-block">
+  <div
+    v-if="filteredMenuItems && filteredMenuItems.length > 0"
+    class="inline-block"
+  >
     <CommonButton
       v-if="singleActionMode"
       :class="buttonVariantClass"
@@ -76,35 +79,35 @@ const buttonVariantClass = computed(() => {
       :icon="singleMenuItem?.icon"
       @click="singleMenuItem?.onClick?.(entity as ObjectLike)"
     />
-    <CommonButton
-      v-else
-      :id="`action-menu-${entityId}`"
-      ref="popoverTarget"
-      :aria-label="$t(customMenuButtonLabel || 'Action menu button')"
-      aria-haspopup="true"
-      :aria-controls="popoverIsOpen ? menuId : undefined"
-      class="text-stone-200 dark:text-neutral-500"
-      :class="{
-        'outline outline-1 outline-offset-1 outline-blue-800': popoverIsOpen,
-      }"
-      :size="buttonSize"
-      icon="three-dots-vertical"
-      @click="toggle"
-    />
-
-    <CommonPopover
-      v-if="!singleActionMode"
-      :id="menuId"
-      ref="popover"
-      :placement="placement"
-      :orientation="orientation"
-      :owner="popoverTarget"
-    >
-      <CommonPopoverMenu
-        ref="popoverMenu"
-        :entity="entity"
-        :popover="popover"
+    <template v-else>
+      <CommonButton
+        :id="`action-menu-${entityId}`"
+        ref="popoverTarget"
+        :aria-label="$t(customMenuButtonLabel || 'Action menu button')"
+        aria-haspopup="true"
+        :aria-controls="popoverIsOpen ? menuId : undefined"
+        class="text-stone-200 dark:text-neutral-500"
+        :class="{
+          'outline outline-1 outline-offset-1 outline-blue-800': popoverIsOpen,
+        }"
+        :size="buttonSize"
+        icon="three-dots-vertical"
+        @click="toggle"
       />
-    </CommonPopover>
+
+      <CommonPopover
+        :id="menuId"
+        ref="popover"
+        :placement="placement"
+        :orientation="orientation"
+        :owner="popoverTarget"
+      >
+        <CommonPopoverMenu
+          ref="popoverMenu"
+          :entity="entity"
+          :popover="popover"
+        />
+      </CommonPopover>
+    </template>
   </div>
 </template>

+ 93 - 53
app/frontend/apps/desktop/components/CommonActionMenu/__tests__/CommonActionMenu.spec.ts

@@ -3,87 +3,127 @@
 import CommonActionMenu from '#desktop/components/CommonActionMenu/CommonActionMenu.vue'
 import renderComponent from '#tests/support/components/renderComponent.ts'
 import type { ObjectLike } from '#shared/types/utils.ts'
+import type { MenuItem } from '#desktop/components/CommonPopover/types.ts'
+import type { Props } from '../CommonActionMenu.vue'
 
 const fn = vi.fn()
-describe('CommonActionMenu', () => {
-  let view: ReturnType<typeof renderComponent>
 
-  const actions = [
+describe('CommonActionMenu', () => {
+  const actions: MenuItem[] = [
     {
       key: 'delete-foo',
       label: 'Delete Foo',
       icon: 'trash3',
-      onClick: ({ id }: { id: string }) => {
-        fn(id)
+      show: () => true,
+      onClick: (entity?: ObjectLike) => {
+        fn(entity?.id)
       },
     },
     {
       key: 'change-foo',
       label: 'Change Foo',
       icon: 'person-gear',
-      onClick: ({ id }: { id: string }) => {
-        fn(id)
+      show: () => true,
+      onClick: (entity?: ObjectLike) => {
+        fn(entity?.id)
       },
     },
   ]
 
-  beforeEach(() => {
-    view = renderComponent(CommonActionMenu, {
+  const renderActionMenu = async (
+    actions: MenuItem[],
+    props?: Partial<Props>,
+  ) => {
+    return renderComponent(CommonActionMenu, {
       props: {
+        ...props,
         entity: {
           id: 'foo-test-action',
         },
         actions,
       },
     })
-  })
+  }
 
-  it('shows action menu button by default', () => {
-    expect(view.getByIconName('three-dots-vertical')).toBeInTheDocument()
-  })
+  describe('Multiple Actions', () => {
+    it('shows action menu button by default', async () => {
+      const view = await renderActionMenu(actions)
+      expect(view.getByIconName('three-dots-vertical')).toBeInTheDocument()
+    })
+
+    it('show not content when no item exists', async () => {
+      const view = await renderActionMenu([
+        {
+          ...actions[0],
+          show: () => false,
+        },
+        {
+          ...actions[1],
+          show: () => false,
+        },
+        {
+          key: 'example',
+          label: 'Example',
+          show: () => false,
+        },
+      ])
+
+      expect(
+        view.queryByIconName('three-dots-vertical'),
+      ).not.toBeInTheDocument()
+    })
 
-  it('calls onClick handler when action is clicked', async () => {
-    await view.events.click(view.getByIconName('three-dots-vertical'))
+    it('calls onClick handler when action is clicked', async () => {
+      const view = await renderActionMenu(actions)
 
-    expect(view.getByIconName('trash3')).toBeInTheDocument()
-    expect(view.getByIconName('person-gear')).toBeInTheDocument()
+      await view.events.click(view.getByIconName('three-dots-vertical'))
 
-    await view.events.click(view.getByText('Change Foo'))
+      expect(view.getByIconName('trash3')).toBeInTheDocument()
+      expect(view.getByIconName('person-gear')).toBeInTheDocument()
 
-    expect(fn).toHaveBeenCalledWith('foo-test-action')
-  })
+      await view.events.click(view.getByText('Change Foo'))
 
-  it('finds corresponding a11y controls', async () => {
-    await view.events.click(view.getByIconName('three-dots-vertical'))
-    const id = view
-      .getByLabelText('Action menu button')
-      .getAttribute('aria-controls')
+      expect(fn).toHaveBeenCalledWith('foo-test-action')
+    })
 
-    const popover = document.getElementById(id as string)
+    it('finds corresponding a11y controls', async () => {
+      const view = await renderActionMenu(actions)
 
-    expect(popover?.getAttribute('id')).toEqual(id)
-  })
+      await view.events.click(view.getByIconName('three-dots-vertical'))
+      const id = view
+        .getByLabelText('Action menu button')
+        .getAttribute('aria-controls')
 
-  it('sets a custom aria label on single action button', async () => {
-    await view.rerender({
-      customMenuButtonLabel: 'Custom Action Menu Label',
+      const popover = document.getElementById(id as string)
+
+      expect(popover?.getAttribute('id')).toEqual(id)
     })
 
-    expect(view.getByLabelText('Custom Action Menu Label')).toBeInTheDocument()
-  })
+    it('sets a custom aria label on single action button', async () => {
+      const view = await renderActionMenu(actions, {
+        customMenuButtonLabel: 'Custom Action Menu Label',
+      })
 
-  describe('single action mode', () => {
-    beforeEach(async () => {
       await view.rerender({
-        actions: [actions[0]],
+        customMenuButtonLabel: 'Custom Action Menu Label',
       })
+
+      expect(
+        view.getByLabelText('Custom Action Menu Label'),
+      ).toBeInTheDocument()
     })
+  })
+
+  describe('single action mode', () => {
+    it('adds aria label on single action button', async () => {
+      const view = await renderActionMenu([actions[0]])
 
-    it('adds aria label on single action button', () => {
       expect(view.getByLabelText('Delete Foo')).toBeInTheDocument()
     })
 
-    it('supports single action mode', () => {
+    it('supports single action mode', async () => {
+      const view = await renderActionMenu([actions[0]])
+
       expect(
         view.queryByIconName('three-dots-vertical'),
       ).not.toBeInTheDocument()
@@ -92,13 +132,15 @@ describe('CommonActionMenu', () => {
     })
 
     it('calls onClick handler when action is clicked', async () => {
+      const view = await renderActionMenu([actions[0]])
+
       await view.events.click(view.getByIconName('trash3'))
 
       expect(fn).toHaveBeenCalledWith('foo-test-action')
     })
 
     it('renders single action if prop is set', async () => {
-      await view.rerender({
+      const view = await renderActionMenu([actions[0]], {
         noSingleActionMode: true,
       })
 
@@ -110,19 +152,17 @@ describe('CommonActionMenu', () => {
     })
 
     it('sets a custom aria label on single action', async () => {
-      await view.rerender({
-        actions: [
-          {
-            key: 'delete-foo',
-            label: 'Delete Foo',
-            ariaLabel: 'Custom Delete Foo',
-            icon: 'trash3',
-            onClick: ({ id }: { id: string }) => {
-              fn(id)
-            },
+      const view = await renderActionMenu([
+        {
+          key: 'delete-foo',
+          label: 'Delete Foo',
+          ariaLabel: 'Custom Delete Foo',
+          icon: 'trash3',
+          onClick: (entity?: ObjectLike) => {
+            fn(entity?.id)
           },
-        ],
-      })
+        },
+      ])
 
       expect(view.getByLabelText('Custom Delete Foo')).toBeInTheDocument()
 
@@ -133,8 +173,8 @@ describe('CommonActionMenu', () => {
             label: 'Delete Foo',
             ariaLabel: (entity: ObjectLike) => `label ${entity.id}`,
             icon: 'trash3',
-            onClick: ({ id }: { id: string }) => {
-              fn(id)
+            onClick: (entity?: ObjectLike) => {
+              fn(entity?.id)
             },
           },
         ],

+ 2 - 0
app/frontend/apps/desktop/components/CommonSimpleTable/__tests__/CommonSimpleTable.spec.ts.snapshot.txt

@@ -98,6 +98,7 @@
         <div
           class="inline-block flex items-center justify-center"
         >
+          
           <button
             aria-haspopup="true"
             aria-label="Action menu button"
@@ -121,6 +122,7 @@
           </button>
           <!--teleport start-->
           <!--teleport end-->
+          
         </div>
       </td>
     </tr>

+ 8 - 3
app/frontend/apps/desktop/entities/two-factor-configuration/composables/useConfigurationTwoFactor.ts

@@ -1,6 +1,6 @@
 // Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
 
-import { computed } from 'vue'
+import { computed, watch } from 'vue'
 import { storeToRefs } from 'pinia'
 
 import SubscriptionHandler from '#shared/server/apollo/handler/SubscriptionHandler.ts'
@@ -30,11 +30,11 @@ export const useConfigurationTwoFactor = () => {
     },
   )
 
-  const userCurrenttwoFactorSubscription = new SubscriptionHandler(
+  const userCurrentTwoFactorSubscription = new SubscriptionHandler(
     useUserCurrentTwoFactorUpdatesSubscription({ userId: session.userId }),
   )
 
-  const userCurrentTwoFactorResult = userCurrenttwoFactorSubscription.result()
+  const userCurrentTwoFactorResult = userCurrentTwoFactorSubscription.result()
 
   const twoFactorConfigurationResult = computed(
     () =>
@@ -86,6 +86,11 @@ export const useConfigurationTwoFactor = () => {
     return Boolean(twoFactorConfigurationResult.value?.recoveryCodesExist)
   })
 
+  // We need to restart the subscription when enabled two factor method list changed.
+  watch(twoFactorEnabledMethods, () =>
+    userCurrentTwoFactorSubscription.operationResult.restart(),
+  )
+
   return {
     twoFactorConfigurationMethods,
     hasEnabledMethods,