From d0d26f6e001f8203af3ca2fd0607cd8c775df7b5 Mon Sep 17 00:00:00 2001
From: Ramesh Mane <101566080+rameshmane7218@users.noreply.github.com>
Date: Tue, 4 Jun 2024 11:13:19 +0530
Subject: [PATCH] fix(nc-gui): pr review changes

---
 packages/nc-gui/components/general/Loader.vue |  8 +++++---
 .../smartsheet/expanded-form/index.vue        | 10 +++-------
 .../components/smartsheet/header/Menu.vue     | 20 +++++++++----------
 3 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/packages/nc-gui/components/general/Loader.vue b/packages/nc-gui/components/general/Loader.vue
index 8e2fc2344b..9096695b14 100644
--- a/packages/nc-gui/components/general/Loader.vue
+++ b/packages/nc-gui/components/general/Loader.vue
@@ -2,7 +2,7 @@
 import { LoadingOutlined } from '@ant-design/icons-vue'
 
 const props = defineProps<{
-  size?: 'small' | 'medium' | 'large' | 'xlarge'
+  size?: 'small' | 'medium' | 'large' | 'xlarge' | 'regular'
   loaderClass?: string
 }>()
 
@@ -18,17 +18,19 @@ function getFontSize() {
       return 'text-xl'
     case 'xlarge':
       return 'text-3xl'
+    case 'regular':
+      return 'text-[16px] leading-4'
   }
 }
 
 const indicator = h(LoadingOutlined, {
-  class: `!${getFontSize()} flex flex-row items-center !bg-inherit !hover:bg-inherit !text-inherit ${props.loaderClass}}`,
+  class: `!${getFontSize()} flex flex-row items-center !bg-inherit !hover:bg-inherit !text-inherit ${props.loaderClass || ''}}`,
   spin: true,
 })
 </script>
 
 <template>
-  <a-spin class="nc-loader flex flex-row items-center" :indicator="indicator" />
+  <a-spin class="nc-loader !flex flex-row items-center" :indicator="indicator" />
 </template>
 
 <style lang="scss" scoped>
diff --git a/packages/nc-gui/components/smartsheet/expanded-form/index.vue b/packages/nc-gui/components/smartsheet/expanded-form/index.vue
index 481f839566..85dc0ac3f9 100644
--- a/packages/nc-gui/components/smartsheet/expanded-form/index.vue
+++ b/packages/nc-gui/components/smartsheet/expanded-form/index.vue
@@ -639,16 +639,12 @@ export default {
             <NcButton
               v-if="!isNew && rowId && !isMobileMode"
               :disabled="isLoading"
-              class="!<lg:hidden text-gray-700 !h-7 !px-2"
+              class="!<lg:hidden text-gray-700 !h-7 !w-7"
               type="text"
               size="xsmall"
               @click="copyRecordUrl()"
             >
-              <div
-                v-e="['c:row-expand:copy-url']"
-                data-testid="nc-expanded-form-copy-url"
-                class="flex items-center"
-              >
+              <div v-e="['c:row-expand:copy-url']" data-testid="nc-expanded-form-copy-url" class="flex items-center">
                 <component :is="iconMap.check" v-if="isRecordLinkCopied" class="cursor-pointer nc-duplicate-row h-4 w-4" />
                 <component :is="iconMap.copy" v-else class="cursor-pointer nc-duplicate-row h-4 w-4" />
               </div>
@@ -888,7 +884,7 @@ export default {
                         class="flex gap-2 items-center"
                         data-testid="nc-expanded-form-copy-url"
                       >
-                        <component :is="iconMap.link" class="cursor-pointer nc-duplicate-row" />
+                        <component :is="iconMap.copy" class="cursor-pointer nc-duplicate-row" />
                         {{ $t('labels.copyRecordURL') }}
                       </div>
                     </NcMenuItem>
diff --git a/packages/nc-gui/components/smartsheet/header/Menu.vue b/packages/nc-gui/components/smartsheet/header/Menu.vue
index 86188ff505..df4c5f8dfc 100644
--- a/packages/nc-gui/components/smartsheet/header/Menu.vue
+++ b/packages/nc-gui/components/smartsheet/header/Menu.vue
@@ -43,7 +43,7 @@ const { gridViewCols } = useViewColumnsOrThrow()
 
 const { fieldsToGroupBy, groupByLimit } = useViewGroupByOrThrow(view)
 
-const isLoading = ref('')
+const isLoading = ref<'' | 'hideOrShow' | 'setDisplay'>('')
 
 const setAsDisplayValue = async () => {
   isLoading.value = 'setDisplay'
@@ -94,8 +94,9 @@ const setAsDisplayValue = async () => {
     })
   } catch (e) {
     message.error(t('msg.error.primaryColumnUpdateFailed'))
+  } finally {
+    isLoading.value = ''
   }
-  isLoading.value = ''
 }
 
 const sortByColumn = async (direction: 'asc' | 'desc') => {
@@ -293,7 +294,8 @@ const hideOrShowField = async () => {
     },
     scope: defineViewScope({ view: view.value }),
   })
-  // isLoading.value = false
+
+  isLoading.value = ''
 }
 
 const handleDelete = () => {
@@ -389,12 +391,8 @@ const filterOrGroupByThisField = (event: SmartsheetStoreEvents) => {
         <a-divider v-if="!column?.pv" class="!my-0" />
         <NcMenuItem v-if="!column?.pv" @click="hideOrShowField">
           <div v-e="['a:field:hide']" class="nc-column-insert-before nc-header-menu-item">
-            <component
-              :is="isHiddenCol ? iconMap.eye : iconMap.eyeSlash"
-              v-show="isLoading !== 'hideOrShow'"
-              class="text-gray-700 !w-3.75 !h-3.75"
-            />
-            <GeneralLoader v-show="isLoading === 'hideOrShow'" size="large" class="!w-3.75 !h-3.75" />
+            <GeneralLoader v-if="isLoading === 'hideOrShow'" size="regular" />
+            <component v-else :is="isHiddenCol ? iconMap.eye : iconMap.eyeSlash" class="text-gray-700 !w-4 !h-4" />
             <!-- Hide Field -->
             {{ isHiddenCol ? $t('general.showField') : $t('general.hideField') }}
           </div>
@@ -404,8 +402,8 @@ const filterOrGroupByThisField = (event: SmartsheetStoreEvents) => {
           @click="setAsDisplayValue"
         >
           <div class="nc-column-set-primary nc-header-menu-item item">
-            <GeneralIcon v-show="isLoading !== 'setDisplay'" icon="star" class="text-gray-700 !w-4.25 !h-4.25" />
-            <GeneralLoader v-show="isLoading === 'setDisplay'" size="large" class="!w-4.25 !h-4.25" />
+            <GeneralLoader v-if="isLoading === 'setDisplay'" size="regular" />
+            <GeneralIcon v-else icon="star" class="text-gray-700 !w-4.25 !h-4.25" />
 
             <!--       todo : tooltip -->
             <!-- Set as Display value -->