review feedback

vatesfr · Jan 24, 2025 · 0973135 · 0973135
1 parent 1fef24a
commit 0973135
Show file tree

Hide file tree

Showing 8 changed files with 50 additions and 48 deletions.
diff --git a/@xen-orchestra/lite/src/components/pool/network/PoolHostInternalNetworksTable.vue b/@xen-orchestra/lite/src/components/pool/network/PoolHostInternalNetworksTable.vue
@@ -57,13 +57,15 @@
               <th v-if="column.id === 'checkbox'" class="checkbox">
                 <UiCheckbox :v-model="areAllSelected" accent="info" @update:model-value="toggleSelect" />
               </th>
-              <th v-else-if="column.id === 'more'" v-tooltip="$t('coming-soon')" class="more">
-                <UiButtonIcon size="small" accent="info" :icon="faEllipsis" />
-                {{ column.label }}
+              <th v-else-if="column.id === 'more'" class="more">
+                <UiButtonIcon v-tooltip="$t('coming-soon')" :icon="faEllipsis" accent="info" disabled size="small" />
+              </th>
+              <th v-else>
+                <div v-tooltip class="text-ellipsis">
+                  <VtsIcon accent="brand" :icon="headerIcon[column.id]" />
+                  {{ column.label }}
+                </div>
               </th>
-              <ColumnTitle v-else :icon="headerIcon[column.id]">
-                <span class="text-ellipsis">{{ column.label }}</span>
-              </ColumnTitle>
             </template>
           </tr>
         </template>
@@ -81,11 +83,13 @@
               :class="{ checkbox: column.id === 'checkbox' }"
             >
               <UiCheckbox v-if="column.id === 'checkbox'" v-model="selected" accent="info" :value="row.id" />
-              <VtsIcon
+              <UiButtonIcon
                 v-else-if="column.id === 'more'"
                 v-tooltip="$t('coming-soon')"
-                accent="info"
                 :icon="faEllipsis"
+                accent="info"
+                disabled
+                size="small"
               />
               <div v-else v-tooltip="{ placement: 'bottom-end' }" class="text-ellipsis">
                 {{ column.value }}
@@ -109,10 +113,9 @@
 <script setup lang="ts">
 import useMultiSelect from '@/composables/multi-select.composable'
 import { useNetworkStore } from '@/stores/xen-api/network.store'
+import VtsDataTable from '@core/components/data-table/VtsDataTable.vue'
 import VtsIcon from '@core/components/icon/VtsIcon.vue'
 import VtsStateHero from '@core/components/state-hero/VtsStateHero.vue'
-import ColumnTitle from '@core/components/table/ColumnTitle.vue'
-import VtsDataTable from '@core/components/table/VtsDataTable.vue'
 import UiButton from '@core/components/ui/button/UiButton.vue'
 import UiButtonIcon from '@core/components/ui/button-icon/UiButtonIcon.vue'
 import UiCheckbox from '@core/components/ui/checkbox/UiCheckbox.vue'
@@ -133,6 +136,7 @@ import {
   faPlus,
   faTrash,
 } from '@fortawesome/free-solid-svg-icons'
+import { noop } from '@vueuse/shared'
 import { computed, ref } from 'vue'
 import { useI18n } from 'vue-i18n'
 
@@ -165,14 +169,14 @@ const getLockingMode = (lockingMode: string) => (lockingMode === 'disabled' ? t(
 const { visibleColumns, rows } = useTable('networks', filteredNetworks, {
   rowId: record => record.uuid,
   columns: define => [
-    define('checkbox', () => '', { label: '', isHideable: false }),
+    define('checkbox', noop, { label: '', isHideable: false }),
     define('name_label', { label: t('name') }),
     define('name_description', { label: t('description') }),
     define('MTU', { label: t('mtu') }),
     define('default_locking_mode', record => getLockingMode(record.default_locking_mode), {
       label: t('default-locking-mode'),
     }),
-    define('more', () => '', { label: '', isHideable: false }),
+    define('more', noop, { label: '', isHideable: false }),
   ],
 })
 

diff --git a/@xen-orchestra/lite/src/components/pool/network/PoolNetworksTable.vue b/@xen-orchestra/lite/src/components/pool/network/PoolNetworksTable.vue
@@ -67,13 +67,15 @@
               <th v-if="column.id === 'checkbox'" class="checkbox">
                 <UiCheckbox :v-model="areAllSelected" accent="info" @update:model-value="toggleSelect" />
               </th>
-              <th v-else-if="column.id === 'more'" v-tooltip="$t('coming-soon')" class="more">
-                <UiButtonIcon size="small" accent="info" :icon="faEllipsis" />
-                {{ column.label }}
+              <th v-else-if="column.id === 'more'" class="more">
+                <UiButtonIcon v-tooltip="$t('coming-soon')" :icon="faEllipsis" accent="info" disabled size="small" />
+              </th>
+              <th v-else>
+                <div v-tooltip class="text-ellipsis">
+                  <VtsIcon accent="brand" :icon="headerIcon[column.id]" />
+                  {{ column.label }}
+                </div>
               </th>
-              <ColumnTitle v-else :icon="headerIcon[column.id]">
-                <span class="text-ellipsis">{{ column.label }}</span>
-              </ColumnTitle>
             </template>
           </tr>
         </template>
@@ -91,11 +93,13 @@
               :class="{ checkbox: column.id === 'checkbox' }"
             >
               <UiCheckbox v-if="column.id === 'checkbox'" v-model="selected" accent="info" :value="row.id" />
-              <VtsIcon
+              <UiButtonIcon
                 v-else-if="column.id === 'more'"
                 v-tooltip="$t('coming-soon')"
-                accent="info"
                 :icon="faEllipsis"
+                accent="info"
+                disabled
+                size="small"
               />
               <VtsConnectionStatus v-else-if="column.id === 'status'" :status="column.value" />
               <div v-else v-tooltip="{ placement: 'bottom-end' }" class="text-ellipsis">
@@ -123,10 +127,9 @@ import type { XenApiNetwork } from '@/libs/xen-api/xen-api.types'
 import { useNetworkStore } from '@/stores/xen-api/network.store'
 import { usePifStore } from '@/stores/xen-api/pif.store'
 import VtsConnectionStatus from '@core/components/connection-status/VtsConnectionStatus.vue'
+import VtsDataTable from '@core/components/data-table/VtsDataTable.vue'
 import VtsIcon from '@core/components/icon/VtsIcon.vue'
 import VtsStateHero from '@core/components/state-hero/VtsStateHero.vue'
-import ColumnTitle from '@core/components/table/ColumnTitle.vue'
-import VtsDataTable from '@core/components/table/VtsDataTable.vue'
 import UiButton from '@core/components/ui/button/UiButton.vue'
 import UiButtonIcon from '@core/components/ui/button-icon/UiButtonIcon.vue'
 import UiCheckbox from '@core/components/ui/checkbox/UiCheckbox.vue'
@@ -150,6 +153,7 @@ import {
   faPowerOff,
   faTrash,
 } from '@fortawesome/free-solid-svg-icons'
+import { noop } from '@vueuse/shared'
 import { computed, ref } from 'vue'
 import { useI18n } from 'vue-i18n'
 
@@ -210,7 +214,7 @@ const getLockingMode = (lockingMode: string) => (lockingMode === 'disabled' ? t(
 const { visibleColumns, rows } = useTable('networks', filteredNetworks, {
   rowId: record => record.uuid,
   columns: define => [
-    define('checkbox', () => '', { label: '', isHideable: false }),
+    define('checkbox', noop, { label: '', isHideable: false }),
     define('name_label', { label: t('name') }),
     define('name_description', { label: t('description') }),
     define('status', record => getNetworkStatus(record), { label: t('pifs-status') }),
@@ -219,7 +223,7 @@ const { visibleColumns, rows } = useTable('networks', filteredNetworks, {
     define('default_locking_mode', record => getLockingMode(record.default_locking_mode), {
       label: t('default-locking-mode'),
     }),
-    define('more', () => '', { label: '', isHideable: false }),
+    define('more', noop, { label: '', isHideable: false }),
   ],
 })
 

diff --git a/@xen-orchestra/lite/src/libs/xen-api/xen-api.types.ts b/@xen-orchestra/lite/src/libs/xen-api/xen-api.types.ts
@@ -59,6 +59,7 @@ type ObjectTypeToRecordMapping = {
   message: XenApiMessage<any>
   network: XenApiNetwork
   pool: XenApiPool
+  pif: XenApiPif
   sr: XenApiSr
   vm: XenApiVm
   vm_guest_metrics: XenApiVmGuestMetrics

diff --git a/@xen-orchestra/lite/src/stores/xen-api/network.store.ts b/@xen-orchestra/lite/src/stores/xen-api/network.store.ts
@@ -17,7 +17,7 @@ export const useNetworkStore = defineStore('xen-api-network', () => {
   const pifContext = deps.pifStore.getContext()
 
   const networksWithPifs = computed(() => {
-    const networkRefs = Array.from(pifContext.pifsByHostMaster.value.keys())
+    const networkRefs = Array.from(pifContext.hostMasterPifsByNetwork.value.keys())
 
     return baseContext.records.value.filter(network => {
       return networkRefs.includes(network.$ref) && network.PIFs.length > 0

diff --git a/@xen-orchestra/lite/src/stores/xen-api/pif.store.ts b/@xen-orchestra/lite/src/stores/xen-api/pif.store.ts
@@ -1,34 +1,32 @@
 import type { XenApiNetwork, XenApiPif } from '@/libs/xen-api/xen-api.types'
 import { createXapiStoreConfig } from '@/stores/xen-api/create-xapi-store-config'
-import { useHostStore } from '@/stores/xen-api/host.store'
 import { usePoolStore } from '@/stores/xen-api/pool.store'
 import { createSubscribableStoreContext } from '@core/utils/create-subscribable-store-context.util'
 import { defineStore } from 'pinia'
 import { computed } from 'vue'
 
 export const usePifStore = defineStore('xen-api-pif', () => {
   const deps = {
-    hostStore: useHostStore(),
     poolStore: usePoolStore(),
   }
   const { context: baseContext, ...configRest } = createXapiStoreConfig('pif')
 
   const poolContext = deps.poolStore.getContext()
 
-  const pifsByHostMaster = computed(() => {
-    const pifsByHostMasterMap = new Map<XenApiNetwork['$ref'], XenApiPif[]>()
+  const hostMasterPifsByNetwork = computed(() => {
+    const hostMasterPifsByNetworkMap = new Map<XenApiNetwork['$ref'], XenApiPif[]>()
 
     baseContext.records.value
-      .filter(pif => poolContext.isPoolMaster(pif.host))
+      .filter(pif => poolContext.isMasterHost(pif.host))
       .forEach(pif => {
         const networkId = pif.network
-        if (!pifsByHostMasterMap.has(networkId)) {
-          pifsByHostMasterMap.set(networkId, [])
+        if (!hostMasterPifsByNetworkMap.has(networkId)) {
+          hostMasterPifsByNetworkMap.set(networkId, [])
         }
-        pifsByHostMasterMap.get(networkId)?.push(pif)
+        hostMasterPifsByNetworkMap.get(networkId)?.push(pif)
       })
 
-    return pifsByHostMasterMap
+    return hostMasterPifsByNetworkMap
   })
 
   const pifsByNetwork = computed(() => {
@@ -45,7 +43,7 @@ export const usePifStore = defineStore('xen-api-pif', () => {
     return pifsByNetworkMap
   })
 
-  const context = { ...baseContext, pifsByNetwork, pifsByHostMaster }
+  const context = { ...baseContext, pifsByNetwork, hostMasterPifsByNetwork }
 
   return createSubscribableStoreContext({ context, ...configRest }, deps)
 })
diff --git a/@xen-orchestra/lite/src/stores/xen-api/pool.store.ts b/@xen-orchestra/lite/src/stores/xen-api/pool.store.ts
@@ -1,4 +1,4 @@
-import type { XenApiPif, XenApiPool } from '@/libs/xen-api/xen-api.types'
+import type { XenApiHost, XenApiPool } from '@/libs/xen-api/xen-api.types'
 import { createXapiStoreConfig } from '@/stores/xen-api/create-xapi-store-config'
 import { createSubscribableStoreContext } from '@core/utils/create-subscribable-store-context.util'
 import { defineStore } from 'pinia'
@@ -9,12 +9,12 @@ export const usePoolStore = defineStore('xen-api-pool', () => {
 
   const pool = computed<XenApiPool | undefined>(() => baseContext.records.value[0])
 
-  const isPoolMaster = (hostRef: XenApiPif['host']) => pool.value?.master === hostRef
+  const isMasterHost = (hostRef: XenApiHost['$ref']) => pool.value?.master === hostRef
 
   const context = {
     ...baseContext,
     pool,
-    isPoolMaster,
+    isMasterHost,
   }
 
   return createSubscribableStoreContext({ context, ...configRest }, {})

diff --git a/@xen-orchestra/lite/src/views/pool/PoolNetworkView.vue b/@xen-orchestra/lite/src/views/pool/PoolNetworkView.vue
@@ -19,8 +19,6 @@ usePageTitleStore().setTitle(useI18n().t('network'))
 
 <style lang="postcss" scoped>
 .pool-network-view {
-  display: flex;
-
   .container {
     display: flex;
     flex-direction: column;

diff --git a/...ore/lib/components/table/VtsDataTable.vue → ...ib/components/data-table/VtsDataTable.vue b/...ore/lib/components/table/VtsDataTable.vue → ...ib/components/data-table/VtsDataTable.vue
@@ -41,21 +41,18 @@ defineSlots<{
   flex-direction: column;
   gap: 0.8rem;
 
-  :deep {
-    tbody tr:hover {
+  :deep(tbody) tr {
+    &:hover {
       cursor: pointer;
       background-color: var(--color-info-background-hover);
     }
-
-    tbody tr:active {
+    &:active {
       background-color: var(--color-info-background-active);
     }
-
-    tbody tr.selected {
+    &.selected {
       background-color: var(--color-info-background-selected);
     }
-
-    tr:last-child {
+    &:last-child {
       border-bottom: 0.1rem solid var(--color-neutral-border);
     }
   }