From fdd9c2be6812d8f968ac583ee5ab66f177166fed Mon Sep 17 00:00:00 2001
From: Ramesh Mane <101566080+rameshmane7218@users.noreply.github.com>
Date: Tue, 27 Feb 2024 12:39:22 +0000
Subject: [PATCH] fix(nc-gui): pr review changes

---
 .../nc-gui/components/cell/MultiSelect.vue    | 10 ++---
 .../nc-gui/components/cell/SingleSelect.vue   |  8 ++--
 packages/nc-gui/components/cell/User.vue      | 41 +++++++++----------
 .../components/general/ImageCropper.vue       |  8 +---
 .../nc-gui/components/smartsheet/Form.vue     |  9 ++--
 .../smartsheet/form/LimitOptions.vue          | 18 ++------
 packages/nc-gui/utils/parseUtils.ts           |  2 +-
 7 files changed, 40 insertions(+), 56 deletions(-)

diff --git a/packages/nc-gui/components/cell/MultiSelect.vue b/packages/nc-gui/components/cell/MultiSelect.vue
index 9559bcb48c..4f0c8346d0 100644
--- a/packages/nc-gui/components/cell/MultiSelect.vue
+++ b/packages/nc-gui/components/cell/MultiSelect.vue
@@ -100,7 +100,7 @@ const options = computed<(SelectOptionType & { value?: string })[]>(() => {
     const limitOptionsById =
       ((parseProp(column.value.meta)?.limitOptions || []).reduce(
         (o: Record<string, FormFieldsLimitOptionsType>, f: FormFieldsLimitOptionsType) => {
-          if (f?.order !== undefined && order < f.order) {
+          if (order < (f?.order ?? 0)) {
             order = f.order
           }
           return {
@@ -118,18 +118,18 @@ const options = computed<(SelectOptionType & { value?: string })[]>(() => {
       (parseProp(column.value.meta)?.limitOptions || []).length
     ) {
       return opts
-        ?.filter((o: SelectOptionType) => {
+        .filter((o: SelectOptionType) => {
           if (limitOptionsById[o.id!]?.show !== undefined) {
             return limitOptionsById[o.id!]?.show
           }
           return false
         })
-        ?.map((o) => ({
+        .map((o) => ({
           ...o,
           value: o.title,
           order: o.id && limitOptionsById[o.id] ? limitOptionsById[o.id]?.order : order++,
         }))
-        ?.sort((a, b) => a.order - b.order)
+        .sort((a, b) => a.order - b.order)
     } else {
       return opts.map((o: SelectOptionType) => ({ ...o, value: o.title }))
     }
@@ -390,7 +390,7 @@ const onFocus = () => {
 <template>
   <div
     class="nc-cell-field nc-multi-select h-full w-full flex items-center"
-    :class="{ 'read-only': readOnly, readOnly, 'max-w-full': isForm }"
+    :class="{ 'read-only': readOnly, 'max-w-full': isForm }"
     @click="toggleMenu"
   >
     <div v-if="!isEditColumn && isForm && parseProp(column.meta)?.isList" class="w-full max-w-full">
diff --git a/packages/nc-gui/components/cell/SingleSelect.vue b/packages/nc-gui/components/cell/SingleSelect.vue
index def1196ca3..aa56a6e4dc 100644
--- a/packages/nc-gui/components/cell/SingleSelect.vue
+++ b/packages/nc-gui/components/cell/SingleSelect.vue
@@ -94,7 +94,7 @@ const options = computed<(SelectOptionType & { value: string })[]>(() => {
     const limitOptionsById =
       ((parseProp(column.value.meta)?.limitOptions || []).reduce(
         (o: Record<string, FormFieldsLimitOptionsType>, f: FormFieldsLimitOptionsType) => {
-          if (f?.order !== undefined && order < f.order) {
+          if (order < (f?.order ?? 0)) {
             order = f.order
           }
           return {
@@ -112,18 +112,18 @@ const options = computed<(SelectOptionType & { value: string })[]>(() => {
       (parseProp(column.value.meta)?.limitOptions || []).length
     ) {
       return opts
-        ?.filter((o: any) => {
+        .filter((o: SelectOptionType & { value: string }) => {
           if (limitOptionsById[o.id]?.show !== undefined) {
             return limitOptionsById[o.id]?.show
           }
           return false
         })
-        ?.map((o: any) => ({
+        .map((o: any) => ({
           ...o,
           value: o.title,
           order: o.id && limitOptionsById[o.id] ? limitOptionsById[o.id]?.order : order++,
         }))
-        ?.sort((a, b) => a.order - b.order)
+        .sort((a, b) => a.order - b.order)
     } else {
       return opts.map((o: any) => ({ ...o, value: o.title }))
     }
diff --git a/packages/nc-gui/components/cell/User.vue b/packages/nc-gui/components/cell/User.vue
index 1764a4f47b..746798c630 100644
--- a/packages/nc-gui/components/cell/User.vue
+++ b/packages/nc-gui/components/cell/User.vue
@@ -83,7 +83,7 @@ const options = computed<UserFieldRecordType[]>(() => {
   const limitOptionsById =
     ((parseProp(column.value.meta)?.limitOptions || []).reduce(
       (o: Record<string, FormFieldsLimitOptionsType>, f: FormFieldsLimitOptionsType) => {
-        if (f?.order !== undefined && order < f.order) {
+        if (order < (f?.order ?? 0)) {
           order = f.order
         }
         return {
@@ -103,34 +103,33 @@ const options = computed<UserFieldRecordType[]>(() => {
     (parseProp(column.value.meta)?.limitOptions || []).length
   ) {
     collaborators.push(
-      ...(
-        baseUsers.value
-          .filter((user) => {
-            if (limitOptionsById[user.id]?.show !== undefined) {
-              return limitOptionsById[user.id]?.show
-            }
-            return false
-          })
-          ?.map((user: any) => ({
-            id: user.id,
-            email: user.email,
-            display_name: user.display_name,
-            deleted: user.deleted,
-            order: user.id && limitOptionsById[user.id] ? limitOptionsById[user.id]?.order ?? user.order : order++,
-          })) || []
-      ).sort((a, b) => a.order - b.order),
+      ...(baseUsers.value || [])
+        .filter((user) => {
+          if (limitOptionsById[user.id]?.show !== undefined) {
+            return limitOptionsById[user.id]?.show
+          }
+          return false
+        })
+        .map((user: any) => ({
+          id: user.id,
+          email: user.email,
+          display_name: user.display_name,
+          deleted: user.deleted,
+          order: user.id && limitOptionsById[user.id] ? limitOptionsById[user.id]?.order ?? user.order : order++,
+        }))
+        .sort((a, b) => a.order - b.order),
     )
   } else {
     collaborators.push(
-      ...(
-        baseUsers.value?.map((user: any) => ({
+      ...(baseUsers.value || [])
+        .map((user: any) => ({
           id: user.id,
           email: user.email,
           display_name: user.display_name,
           deleted: user.deleted,
           order: order++,
-        })) || []
-      ).sort((a, b) => a.order - b.order),
+        }))
+        .sort((a, b) => a.order - b.order),
     )
   }
   return collaborators
diff --git a/packages/nc-gui/components/general/ImageCropper.vue b/packages/nc-gui/components/general/ImageCropper.vue
index fa0d6f4616..3c17460fbd 100644
--- a/packages/nc-gui/components/general/ImageCropper.vue
+++ b/packages/nc-gui/components/general/ImageCropper.vue
@@ -103,13 +103,7 @@ watch(showCropper, () => {
         class="nc-cropper relative"
         :src="imageConfig.src"
         :auto-zoom="true"
-        :stencil-props="
-          cropperConfig?.aspectRatio
-            ? {
-                aspectRatio: cropperConfig.aspectRatio,
-              }
-            : {}
-        "
+        :stencil-props="cropperConfig?.aspectRatio ? { aspectRatio: cropperConfig.aspectRatio } : {}"
       />
       <div v-if="previewImage.src" class="result_preview">
         <img :src="previewImage.src" alt="Preview Image" />
diff --git a/packages/nc-gui/components/smartsheet/Form.vue b/packages/nc-gui/components/smartsheet/Form.vue
index 93570e84ed..8234c0b7d5 100644
--- a/packages/nc-gui/components/smartsheet/Form.vue
+++ b/packages/nc-gui/components/smartsheet/Form.vue
@@ -170,6 +170,7 @@ const { betaFeatureToggleState } = useBetaFeatureToggle()
 const { open, onChange: onChangeFile } = useFileDialog({
   accept: 'image/*',
   multiple: false,
+  reset: true,
 })
 
 const visibleColumns = computed(() => localColumns.value.filter((f) => f.show).sort((a, b) => a.order - b.order))
@@ -695,7 +696,7 @@ useEventListener(
             ></GeneralImageCropper>
             <!-- cover image -->
             <div class="group relative max-w-[max(33%,688px)] mx-auto">
-              <GeneralFormBanner :banner-image-url="formViewData.banner_image_url"/>
+              <GeneralFormBanner :banner-image-url="formViewData.banner_image_url" />
               <div class="absolute bottom-0 right-0 hidden group-hover:block">
                 <div class="flex items-center space-x-1 m-2">
                   <NcButton
@@ -750,8 +751,8 @@ useEventListener(
                   <!-- Form logo  -->
                   <div class="mb-4">
                     <div
-                      class="nc-form-logo-wrapper mx-6 group relative rounded-xl inline-block h-56px overflow-hidden"
-                      :class="formViewData.logo_url ? 'hover:(w-full bg-gray-100) max-w-189px' : 'w-full bg-gray-100 max-w-148px'"
+                      class="nc-form-logo-wrapper mx-6 group relative rounded-xl inline-block h-56px max-w-189px overflow-hidden"
+                      :class="formViewData.logo_url ? 'hover:(w-full bg-gray-100)' : 'bg-gray-100'"
                       style="transition: all 0.3s ease-in"
                     >
                       <LazyCellAttachmentImage
@@ -761,7 +762,7 @@ useEventListener(
                       />
                       <div
                         class="items-center space-x-1 flex-nowrap m-3"
-                        :class="'hidden absolute top-0 left-0 group-hover:flex'"
+                        :class="formViewData.logo_url ? 'hidden absolute top-0 left-0 group-hover:flex' : 'flex'"
                       >
                         <NcButton
                           v-if="isEditable"
diff --git a/packages/nc-gui/components/smartsheet/form/LimitOptions.vue b/packages/nc-gui/components/smartsheet/form/LimitOptions.vue
index 85a3c0e50b..730275a51f 100644
--- a/packages/nc-gui/components/smartsheet/form/LimitOptions.vue
+++ b/packages/nc-gui/components/smartsheet/form/LimitOptions.vue
@@ -32,7 +32,7 @@ const vModel = computed({
     let order = 1
     const limitOptionsById =
       (props.modelValue || []).reduce((o: Record<string, FormFieldsLimitOptionsType>, f: FormFieldsLimitOptionsType) => {
-        if (f?.order !== undefined && order < f.order) {
+        if (order < (f?.order ?? 0)) {
           order = f.order
         }
         return {
@@ -43,12 +43,12 @@ const vModel = computed({
 
     if (UITypes.User === column.value.uidt) {
       const collaborators = ((baseUsers.value || []) as UserFieldRecordType[])
-        ?.filter((user) => !user?.deleted)
-        ?.map((user: any) => ({
+        .filter((user) => !user?.deleted)
+        .map((user: any) => ({
           id: user.id,
           email: user.email,
           display_name: user.display_name,
-          order: user.id && limitOptionsById[user.id] ? limitOptionsById[user.id]?.order ?? c.order : order++,
+          order: user.id && limitOptionsById[user.id] ? limitOptionsById[user.id]?.order ?? user.order : order++,
           show: user.id && limitOptionsById[user.id] ? limitOptionsById[user.id]?.show : !(props.modelValue || []).length,
         }))
         .sort((a, b) => a.order - b.order)
@@ -99,16 +99,6 @@ async function onMove(_event: { moved: { newIndex: number; oldIndex: number; ele
     moved: { newIndex = 0, oldIndex = 0, element },
   } = _event
 
-  if (!vModel.value.length || vModel.value.length === 1) {
-    element.order = 1
-  } else if (vModel.value.length - 1 === newIndex) {
-    element.order = (vModel.value[newIndex - 1]?.order || 0) + 1
-  } else if (newIndex === 0) {
-    element.order = (vModel.value[1]?.order || 0) / 2
-  } else {
-    element.order = ((vModel.value[newIndex - 1]?.order || 0) + (vModel.value[newIndex + 1].order || 0)) / 2
-  }
-
   let nextOrder: number
 
   // set new order value based on the new order of the items
diff --git a/packages/nc-gui/utils/parseUtils.ts b/packages/nc-gui/utils/parseUtils.ts
index ae567ab3b9..7e89c6dd24 100644
--- a/packages/nc-gui/utils/parseUtils.ts
+++ b/packages/nc-gui/utils/parseUtils.ts
@@ -1,7 +1,7 @@
 export function parseProp(v: any): any {
   if (!v) return {}
   try {
-    return typeof v === 'string' ? JSON.parse(v) : v
+    return typeof v === 'string' ? JSON.parse(v) ?? {} : v
   } catch {
     return {}
   }