Merge pull request #5918 from nocodb/fix/various-multi-select

fix: various fixes for UI

packages/nc-gui/components/cell/MultiSelect.vue

@@ -316,10 +316,10 @@ const handleClose = (e: MouseEvent) => {
 useEventListener(document, 'click', handleClose, true)
 
 const selectedOpts = computed(() => {
-  return options.value.reduce<(SelectOptionType & { index: number })[]>((selectedOptions, option) => {
+  return vModel.value.reduce<SelectOptionType[]>((selectedOptions, option) => {
-    const index = vModel.value.indexOf(option.value!)
+    const selectedOption = options.value.find((o) => o.value === option)
-    if (index !== -1) {
+    if (selectedOption) {
-      selectedOptions.push({ ...option, index })
+      selectedOptions.push(selectedOption)
     }
     return selectedOptions
   }, [])
@@ -340,7 +340,7 @@ const selectedOpts = computed(() => {
       }"
     >
       <template v-for="selectedOpt of selectedOpts" :key="selectedOpt.value">
-        <a-tag class="rounded-tag" :color="selectedOpt.color" :style="{ order: selectedOpt.index }">
+        <a-tag class="rounded-tag" :color="selectedOpt.color">
           <span
             :style="{
               'color': tinycolor.isReadable(selectedOpt.color || '#ccc', '#fff', { level: 'AA', size: 'large' })

packages/nc-gui/components/cell/SingleSelect.vue

@@ -8,7 +8,6 @@ import {
   ActiveCellInj,
   CellClickHookInj,
   ColumnInj,
-  EditModeInj,
   IsFormInj,
   IsKanbanInj,
   ReadonlyInj,

packages/nc-gui/components/smartsheet/Grid.vue

@@ -97,6 +97,7 @@ const contextMenu = computed({
     }
   },
 })
+const contextMenuClosing = ref(false)
 
 const routeQuery = $computed(() => route.query as Record<string, string>)
 const contextMenuTarget = ref<{ row: number; col: number } | null>(null)
@@ -209,6 +210,7 @@ const {
   data,
   $$(editEnabled),
   isPkAvail,
+  contextMenu,
   clearCell,
   clearSelectedRangeOfCells,
   makeEditable,
@@ -479,7 +481,10 @@ defineExpose({
 // reset context menu target on hide
 watch(contextMenu, () => {
   if (!contextMenu.value) {
+    contextMenuClosing.value = true
     contextMenuTarget.value = null
+  } else {
+    contextMenuClosing.value = false
   }
 })
 
@@ -1041,7 +1046,7 @@ function addEmptyRow(row?: number) {
                     :data-col="columnObj.id"
                     :data-title="columnObj.title"
                     @mousedown="handleMouseDown($event, rowIndex, colIndex)"
-                    @mouseover="handleMouseOver(rowIndex, colIndex)"
+                    @mouseover="handleMouseOver($event, rowIndex, colIndex)"
                     @click="handleCellClick($event, rowIndex, colIndex)"
                     @dblclick="makeEditable(row, columnObj)"
                     @contextmenu="showContextMenu($event, { row: rowIndex, col: colIndex })"
@@ -1115,7 +1120,7 @@ function addEmptyRow(row?: number) {
               </div>
             </a-menu-item>
 
-            <a-menu-item v-if="data.some((r) => r.rowMeta.selected)" @click="deleteSelectedRows">
+            <a-menu-item v-if="!contextMenuClosing && data.some((r) => r.rowMeta.selected)" @click="deleteSelectedRows">
               <div v-e="['a:row:delete-bulk']" class="nc-project-menu-item">
                 <!-- Delete Selected Rows -->
                 {{ $t('activity.deleteSelectedRow') }}
@@ -1136,7 +1141,7 @@ function addEmptyRow(row?: number) {
             </a-menu-item>
 
             <!--            Clear cell -->
-            <a-menu-item v-else @click="clearSelectedRangeOfCells()">
+            <a-menu-item v-else-if="contextMenuTarget" @click="clearSelectedRangeOfCells()">
               <div v-e="['a:row:clear-range']" class="nc-project-menu-item">Clear Cells</div>
             </a-menu-item>
 

packages/nc-gui/composables/useMultiSelect/index.ts

@@ -1,4 +1,5 @@
 import dayjs from 'dayjs'
+import type { Ref } from 'vue'
 import type { MaybeRef } from '@vueuse/core'
 import type { ColumnType, LinkToAnotherRecordType, TableType } from 'nocodb-sdk'
 import { RelationTypes, UITypes, isSystemColumn, isVirtualCol } from 'nocodb-sdk'
@@ -39,6 +40,7 @@ export function useMultiSelect(
   data: MaybeRef<Row[]>,
   _editEnabled: MaybeRef<boolean>,
   isPkAvail: MaybeRef<boolean | undefined>,
+  contextMenu: Ref<boolean>,
   clearCell: Function,
   clearSelectedRangeOfCells: Function,
   makeEditable: Function,
@@ -81,6 +83,9 @@ export function useMultiSelect(
       return
     }
 
+    // disable edit mode if active cell is changed
+    editEnabled.value = false
+
     activeCell.row = row
     activeCell.col = col
   }
@@ -207,13 +212,6 @@ export function useMultiSelect(
     }
   }
 
-  function handleMouseOver(row: number, col: number) {
-    if (!isMouseDown) {
-      return
-    }
-    selectedRange.endRange({ row, col })
-  }
-
   function isCellSelected(row: number, col: number) {
     if (activeCell.col === col && activeCell.row === row) {
       return true
@@ -231,6 +229,19 @@ export function useMultiSelect(
     )
   }
 
+  function handleMouseOver(event: MouseEvent, row: number, col: number) {
+    if (!isMouseDown) {
+      return
+    }
+
+    // extend the selection and scroll to the cell
+    selectedRange.endRange({ row, col })
+    scrollToCell?.(row, col)
+
+    // avoid selecting text
+    event.preventDefault()
+  }
+
   function handleMouseDown(event: MouseEvent, row: number, col: number) {
     // if there was a right click on selected range, don't restart the selection
     if (
@@ -240,11 +251,26 @@ export function useMultiSelect(
       return
     }
 
-    isMouseDown = true
+    // if edit is enabled, don't start the selection (some cells shrink after edit mode, which causes the selection to expand if flag is set)
+    if (!editEnabled.value) isMouseDown = true
+
+    contextMenu.value = false
+
+    // avoid text selection
+    event.preventDefault()
+
+    // if shift key is pressed, extend the selection
+    if (event.shiftKey) {
+      // if shift key is pressed, don't restart the selection (unless there is no active cell)
+      if (activeCell.col === null || activeCell.row === null) {
+        selectedRange.startRange({ row, col })
+      }
 
-    // if shift key is pressed, don't restart the selection
+      selectedRange.endRange({ row, col })
-    if (event.shiftKey) return
+      return
+    }
 
+    // start a new selection
     selectedRange.startRange({ row, col })
 
     if (activeCell.row !== row || activeCell.col !== col) {
@@ -255,42 +281,26 @@ export function useMultiSelect(
   }
 
   const handleCellClick = (event: MouseEvent, row: number, col: number) => {
-    isMouseDown = true
+    // if shift key is pressed, don't change the active cell (unless there is no active cell)
-
-    // if shift key is pressed, prevent selecting text
-    if (event.shiftKey && !unref(editEnabled)) {
-      event.preventDefault()
-    }
-
-    // if shift key is pressed, don't restart the selection (unless there is no active cell)
     if (!event.shiftKey || activeCell.col === null || activeCell.row === null) {
-      selectedRange.startRange({ row, col })
       makeActive(row, col)
     }
 
-    selectedRange.endRange({ row, col })
     scrollToCell?.(row, col)
-    isMouseDown = false
   }
 
-  const handleMouseUp = (event: MouseEvent) => {
+  const handleMouseUp = (_event: MouseEvent) => {
     if (isMouseDown) {
       isMouseDown = false
-
       // timeout is needed, because we want to set cell as active AFTER all the child's click handler's called
       // this is needed e.g. for date field edit, where two clicks had to be done - one to select cell, and another one to open date dropdown
       setTimeout(() => {
-        // if shift key is pressed, don't change the active cell
-        if (event.shiftKey) return
         if (selectedRange._start) {
-          makeActive(selectedRange._start.row, selectedRange._start.col)
+          if (activeCell.row !== selectedRange._start.row || activeCell.col !== selectedRange._start.col) {
+            makeActive(selectedRange._start.row, selectedRange._start.col)
+          }
         }
       }, 0)
-
-      // if the editEnabled is false, prevent selecting text on mouseUp
-      if (!unref(editEnabled)) {
-        event.preventDefault()
-      }
     }
   }