Browse Source

Feature: Desktop view - Added additional loading improvements for the checklists.

Dominik Klein 6 months ago
parent
commit
ae1912cc07

+ 5 - 1
app/assets/javascripts/app/views/ticket_zoom/sidebar_checklist_show_row.jst.eco

@@ -40,7 +40,11 @@
           </label>
 
         <div class="checklistItemValue <% if !@readOnly: %>js-checklist-item-edit u-clickable<% end %>" <% if !@readOnly: %>data-table-action="edit"<% end %>>
-          <%- App.Utils.linkify(@object.text) || '-' %>
+          <% if @object.text: %>
+            <%- App.Utils.linkify(@object.text) %>
+          <% else if @readonly: %>
+            <%- '—' %>
+          <% end %>
         </div>
       <% end %>
 

+ 10 - 2
app/frontend/apps/desktop/pages/ticket/__tests__/ticket-detail-view.spec.ts

@@ -227,6 +227,7 @@ describe('ticket detail view', () => {
 
     mockTicketChecklistQuery({
       ticketChecklist: {
+        id: convertToGraphQLId('Checklist', 1),
         name: 'Checklist title',
         items: [
           { text: 'Item 1', checked: true, ticketAccess: null, ticket: null },
@@ -247,6 +248,7 @@ describe('ticket detail view', () => {
     await getTicketChecklistUpdatesSubscriptionHandler().trigger({
       ticketChecklistUpdates: {
         ticketChecklist: {
+          id: convertToGraphQLId('Checklist', 1),
           name: 'Checklist title',
           items: [
             { text: 'Item 1', checked: true },
@@ -257,10 +259,16 @@ describe('ticket detail view', () => {
       },
     })
 
-    await view.events.click(checklistCheckboxes[0])
-
     expect(
       view.queryByRole('status', { name: 'Incomplete checklist items' }),
     ).not.toBeInTheDocument()
+
+    // Click manually in the frontend again on one of the checklist to show
+    // the incomplete state again(without a subscription = manual cache update).
+    await view.events.click(checklistCheckboxes[1])
+
+    expect(
+      await view.findByRole('status', { name: 'Incomplete checklist items' }),
+    ).toBeInTheDocument()
   })
 })

+ 4 - 10
app/frontend/apps/desktop/pages/ticket/components/TicketSidebar.vue

@@ -13,10 +13,6 @@ interface Props {
   context: TicketSidebarContext
   isCollapsed: boolean
   toggleCollapse: () => void
-  /**
-   * List of component names that should be cached by the `KeepAlive` component.
-   * */
-  cache?: string[]
 }
 
 const props = defineProps<Props>()
@@ -44,12 +40,10 @@ const maybeToggleAndSwitchSidebar = (newSidebar: string) => {
       :loading="!activeSidebarPlugin?.contentComponent"
     >
       <div v-show="!isCollapsed" class="flex grow flex-col">
-        <KeepAlive :include="cache || []">
-          <component
-            :is="activeSidebarPlugin?.contentComponent"
-            :context="context"
-          />
-        </KeepAlive>
+        <component
+          :is="activeSidebarPlugin?.contentComponent"
+          :context="context"
+        />
       </div>
     </CommonLoader>
     <div

+ 0 - 1
app/frontend/apps/desktop/pages/ticket/components/TicketSidebar/TicketSidebarChecklist/TicketSidebarChecklistButton.vue

@@ -30,7 +30,6 @@ const badge = computed<TicketSidebarButtonBadgeDetails | undefined>(() => {
   }
 })
 
-// TODO: Check if it's correct not to have any condition for showing the sidebar.
 onMounted(() => {
   emit('show')
 })

+ 6 - 7
app/frontend/apps/desktop/pages/ticket/components/TicketSidebar/TicketSidebarChecklist/TicketSidebarChecklistContent.vue

@@ -36,10 +36,6 @@ import { useTicketChecklistItemOrderUpdateMutation } from '#desktop/pages/ticket
 import { useTicketChecklistItemUpsertMutation } from '#desktop/pages/ticket/graphql/mutations/ticketChecklistItemUpsert.api.ts'
 import { useTicketChecklistTitleUpdateMutation } from '#desktop/pages/ticket/graphql/mutations/ticketChecklistTitleUpdate.api.ts'
 
-defineOptions({
-  name: 'ChecklistContent',
-})
-
 defineProps<TicketSidebarContentProps>()
 
 const checklistItemsComponent = ref<InstanceType<typeof ChecklistItemsType>>()
@@ -54,7 +50,10 @@ const onSubscriptionUpdateCallback = (
   const index = findChangedIndex(previousChecklist, newChecklist)
 
   if (index >= 0)
-    nextTick(() => checklistItemsComponent.value?.quitItemEditing(index))
+    nextTick(() => {
+      checklistItemsComponent.value?.quitReordering()
+      checklistItemsComponent.value?.quitItemEditing(index)
+    })
 }
 
 const { checklist, isLoadingChecklist, readOnly } = useTicketChecklist(
@@ -393,7 +392,7 @@ const { isLoadingTemplates, checklistTemplatesMenuItems } =
     :title="__('Checklist')"
     icon="checklist"
   >
-    <CommonLoader :loading="isLoadingChecklist || isLoadingTemplates">
+    <CommonLoader :loading="isLoadingChecklist">
       <div class="flex flex-col gap-3">
         <ChecklistItems
           v-if="checklist"
@@ -426,7 +425,7 @@ const { isLoadingTemplates, checklistTemplatesMenuItems } =
             "
             :templates="checklistTemplatesMenuItems"
           />
-          <ChecklistEmptyTemplates v-else />
+          <ChecklistEmptyTemplates v-else-if="!isLoadingTemplates" />
         </template>
         <CommonLabel v-else>{{
           $t('No checklist added to this ticket yet.')

+ 2 - 0
app/frontend/apps/desktop/pages/ticket/components/TicketSidebar/TicketSidebarChecklist/TicketSidebarChecklistContent/ChecklistItems.vue

@@ -56,6 +56,7 @@ dragAndDrop({
   draggable: (el) => {
     // Library bug: The draggable attribute is not set always
     // Workaround to set the attribute manually
+    // https://github.com/formkit/drag-and-drop/issues/96
     el.setAttribute('draggable', isReordering.value.toString())
     return isReordering.value
   },
@@ -93,6 +94,7 @@ defineExpose({
   focusTitle: () => checklistTitleComponent.value?.activateEditing(),
   quitItemEditing: (index: number) =>
     checklistNodes.value?.at(index)?.quitEditing(),
+  quitReordering: resetOrder,
   focusNewItem,
 })
 </script>

+ 8 - 1
app/frontend/apps/desktop/pages/ticket/components/TicketSidebar/TicketSidebarChecklist/useChecklistTemplates.ts

@@ -34,9 +34,16 @@ export const useChecklistTemplates = (
     ),
   )
 
-  const isLoadingTemplates = checklistTemplatesQuery.loading()
+  const templatesLoading = checklistTemplatesQuery.loading()
   const checklistTemplates = checklistTemplatesQuery.result()
 
+  const isLoadingTemplates = computed(() => {
+    // Return already true when an templates exists already in the cache.
+    if (checklistTemplates.value !== undefined) return false
+
+    return templatesLoading.value
+  })
+
   checklistTemplatesQuery.subscribeToMore<
     ChecklistTemplateUpdatesSubscriptionVariables,
     ChecklistTemplateUpdatesSubscription

+ 24 - 11
app/frontend/apps/desktop/pages/ticket/components/TicketSidebar/TicketSidebarChecklist/useTicketChecklist.ts

@@ -26,24 +26,27 @@ export const useTicketChecklist = (
   const readOnly = computed(() => !ticket.value?.policy.update)
 
   const checklistQuery = new QueryHandler(
-    useTicketChecklistQuery(
-      () => ({
-        ticketId: ticketId.value,
-      }),
-      {
-        fetchPolicy: 'cache-and-network',
-      },
-    ),
+    useTicketChecklistQuery(() => ({
+      ticketId: ticketId.value,
+    })),
     {
       errorCallback: (error) => error.type !== GraphQLErrorTypes.RecordNotFound,
     },
   )
 
   const checklistResult = checklistQuery.result()
-  const isLoadingChecklist = checklistQuery.loading()
+  const checklistLoading = checklistQuery.loading()
 
   const checklist = computed(() => checklistResult.value?.ticketChecklist)
 
+  const isLoadingChecklist = computed(() => {
+    // Return already true when an checklist result already exists from the cache, also
+    // when maybe a loading is in progress(because of cache + network).
+    if (checklist.value !== undefined) return false
+
+    return checklistLoading.value
+  })
+
   const incompleteItemCount = computed(
     () => checklistResult.value?.ticketChecklist?.incomplete,
   )
@@ -72,9 +75,19 @@ export const useTicketChecklist = (
           ticketChecklist.items as ChecklistItem[],
         )
 
-      return {
-        ticketChecklist,
+      // When a complete checklist was removed, we need to update the result.
+      if (
+        ticketChecklist === null ||
+        (prev.ticketChecklist === null && ticketChecklist !== null)
+      ) {
+        return {
+          ticketChecklist,
+        }
       }
+
+      // Always return null, because we need not to manipulate the data with this function. It's only for handling the
+      //  edit mode callback.
+      return null as unknown as TicketChecklistQuery
     },
   }))
 

+ 1 - 2
app/frontend/apps/desktop/pages/ticket/views/TicketDetailView.vue

@@ -75,7 +75,7 @@ const { hasSidebar } = useTicketSidebar(sidebarContext)
       <div class="relative flex w-full flex-col">
         <TicketDetailTopBar />
 
-        <ArticleList :aria-busy="isLoadingArticles.value" />
+        <ArticleList :aria-busy="isLoadingArticles" />
       </div>
     </CommonLoader>
 
@@ -84,7 +84,6 @@ const { hasSidebar } = useTicketSidebar(sidebarContext)
         :is-collapsed="isCollapsed"
         :toggle-collapse="toggleCollapse"
         :context="sidebarContext"
-        :cache="['ChecklistContent']"
       />
     </template>
   </LayoutContent>

+ 6 - 9
app/frontend/shared/entities/ticket-article/composables/useArticleDataHandler.ts

@@ -43,14 +43,11 @@ export const useArticleDataHandler = (
     options.firstArticlesCount ? toValue(options.firstArticlesCount) : 5,
   )
   const articlesQuery = new QueryHandler(
-    useTicketArticlesQuery(
-      () => ({
-        ticketId: graphqlTicketId.value,
-        pageSize: options.pageSize || 20,
-        firstArticlesCount: firstArticlesCount.value,
-      }),
-      { fetchPolicy: 'cache-and-network' },
-    ),
+    useTicketArticlesQuery(() => ({
+      ticketId: graphqlTicketId.value,
+      pageSize: options.pageSize || 20,
+      firstArticlesCount: firstArticlesCount.value,
+    })),
   )
 
   const articleResult = articlesQuery.result()
@@ -70,7 +67,7 @@ export const useArticleDataHandler = (
     })
   }
 
-  const isLoadingArticles = computed(() => articlesQuery.loading())
+  const isLoadingArticles = articlesQuery.loading()
 
   const adjustPageInfoAfterDeletion = (nextEndCursorEdge?: Maybe<string>) => {
     const newPageInfo: Pick<PageInfo, 'startCursor' | 'endCursor'> = {}

Some files were not shown because too many files changed in this diff