From 7cfa6491fcc03070a22323c766f14e29a388d381 Mon Sep 17 00:00:00 2001
From: Pranav C <pranavxc@gmail.com>
Date: Mon, 31 Oct 2022 11:24:46 +0530
Subject: [PATCH] feat(gui): On first click outside checkbox, just select cell.
 Do not toggle value

Signed-off-by: Pranav C <pranavxc@gmail.com>
---
 packages/nc-gui/components/cell/Checkbox.vue  | 21 ++++++++++++++-----
 .../nc-gui/components/cell/MultiSelect.vue    | 12 +++++------
 .../nc-gui/components/cell/SingleSelect.vue   |  6 +++---
 .../nc-gui/components/smartsheet/Grid.vue     | 15 +++++++++++--
 .../composables/useMultiSelect/index.ts       |  6 ++++++
 5 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/packages/nc-gui/components/cell/Checkbox.vue b/packages/nc-gui/components/cell/Checkbox.vue
index f5d76f8703..cfc3013cf5 100644
--- a/packages/nc-gui/components/cell/Checkbox.vue
+++ b/packages/nc-gui/components/cell/Checkbox.vue
@@ -1,5 +1,13 @@
 <script setup lang="ts">
-import { ActiveCellInj, ColumnInj, IsFormInj, ReadonlyInj, getMdiIcon, inject, useSelectedCellKeyupListener } from '#imports'
+import {
+  ActiveCellInj,
+  ColumnInj,
+  IsFormInj,
+  ReadonlyInj,
+  getMdiIcon,
+  inject,
+  useSelectedCellKeyupListener,
+} from '#imports'
 
 interface Props {
   // If the previous cell value was a text, the initial checkbox value is a string type
@@ -15,6 +23,8 @@ const props = defineProps<Props>()
 
 const emits = defineEmits<Emits>()
 
+const active = inject(ActiveCellInj, ref(false))
+
 let vModel = $computed({
   get: () => !!props.modelValue && props.modelValue !== '0' && props.modelValue !== 0,
   set: (val) => emits('update:modelValue', val),
@@ -37,13 +47,13 @@ const checkboxMeta = $computed(() => {
   }
 })
 
-function onClick() {
-  if (!readOnly?.value) {
+function onClick(force?: boolean) {
+  if (!readOnly?.value && (force || active.value)) {
     vModel = !vModel
   }
 }
 
-useSelectedCellKeyupListener(inject(ActiveCellInj, ref(false)), (e) => {
+useSelectedCellKeyupListener(active, (e) => {
   switch (e.key) {
     case 'Enter':
       onClick()
@@ -62,7 +72,7 @@ useSelectedCellKeyupListener(inject(ActiveCellInj, ref(false)), (e) => {
       'nc-cell-hover-show': !vModel && !readOnly,
       'opacity-0': readOnly && !vModel,
     }"
-    @click="onClick"
+    @click="onClick(false)"
   >
     <div class="px-1 pt-1 rounded-full items-center" :class="{ 'bg-gray-100': !vModel, '!ml-[-8px]': readOnly }">
       <Transition name="layout" mode="out-in" :duration="100">
@@ -71,6 +81,7 @@ useSelectedCellKeyupListener(inject(ActiveCellInj, ref(false)), (e) => {
           :style="{
             color: checkboxMeta.color,
           }"
+          @click.stop="onClick(true)"
         />
       </Transition>
     </div>
diff --git a/packages/nc-gui/components/cell/MultiSelect.vue b/packages/nc-gui/components/cell/MultiSelect.vue
index ecca3abdd7..537ad815a7 100644
--- a/packages/nc-gui/components/cell/MultiSelect.vue
+++ b/packages/nc-gui/components/cell/MultiSelect.vue
@@ -94,7 +94,7 @@ const handleKeys = (e: KeyboardEvent) => {
       break
     case 'ArrowDown':
     case 'ArrowUp':
-      e.stopPropagation()
+      if (isOpen.value) e.stopPropagation()
       break
     case 'Enter':
       isOpen.value = true
@@ -141,9 +141,9 @@ watch(isOpen, (n, _o) => {
 })
 
 useSelectedCellKeyupListener(active, (e) => {
-  if (e.key === 'Enter') {
-    e.stopPropagation()
-    isOpen.value = true
+  if (e.key === 'Escape') {
+    // e.stopPropagation()
+    isOpen.value = false
   }
 })
 </script>
@@ -160,9 +160,9 @@ useSelectedCellKeyupListener(active, (e) => {
     :open="isOpen"
     :disabled="readOnly"
     :class="{ '!ml-[-8px]': readOnly }"
-    dropdown-class-name="nc-dropdown-multi-select-cell"
+    :dropdown-class-name="`nc-dropdown-multi-select-cell ${isOpen ? 'active' : ''}`"
     @keydown="handleKeys"
-    @click="isOpen = !isOpen"
+    @click="isOpen = active && !isOpen"
   >
     <a-select-option
       v-for="op of options"
diff --git a/packages/nc-gui/components/cell/SingleSelect.vue b/packages/nc-gui/components/cell/SingleSelect.vue
index c46e34adc1..f03bd095f9 100644
--- a/packages/nc-gui/components/cell/SingleSelect.vue
+++ b/packages/nc-gui/components/cell/SingleSelect.vue
@@ -52,7 +52,7 @@ const handleKeys = (e: KeyboardEvent) => {
       break
     case 'ArrowDown':
     case 'ArrowUp':
-      e.stopPropagation()
+      if (isOpen.value) e.stopPropagation()
       break
     case 'Enter':
       isOpen.value = true
@@ -85,10 +85,10 @@ watch(isOpen, (n, _o) => {
     :open="isOpen"
     :disabled="readOnly"
     :show-arrow="!readOnly && (active || vModel === null)"
-    dropdown-class-name="nc-dropdown-single-select-cell"
+    :dropdown-class-name="`nc-dropdown-single-select-cell ${isOpen ? 'active' : ''}`"
     @select="isOpen = false"
     @keydown="handleKeys"
-    @click="isOpen = !isOpen"
+    @click="isOpen = active && !isOpen"
   >
     <a-select-option
       v-for="op of options"
diff --git a/packages/nc-gui/components/smartsheet/Grid.vue b/packages/nc-gui/components/smartsheet/Grid.vue
index eea6425bd1..912802d3cc 100644
--- a/packages/nc-gui/components/smartsheet/Grid.vue
+++ b/packages/nc-gui/components/smartsheet/Grid.vue
@@ -213,6 +213,16 @@ const { selectCell, selectBlock, selectedRange, clearRangeRows, startSelectRange
       })
     }
   },
+  (e: KeyboardEvent) => {
+    console.log(e)
+    if (e.code === 'Space') {
+      if (selected.row !== null && !editEnabled) {
+        const row = data.value[selected.row]
+        expandForm(row)
+        return true
+      }
+    }
+  },
 )
 
 onMounted(loadGridViewColumns)
@@ -234,7 +244,7 @@ const showLoading = ref(true)
 
 const skipRowRemovalOnCancel = ref(false)
 
-const expandForm = (row: Row, state?: Record<string, any>, fromToolbar = false) => {
+function expandForm(row: Row, state?: Record<string, any>, fromToolbar = false) {
   const rowId = extractPkFromRow(row.row, meta.value?.columns as ColumnType[])
 
   if (rowId) {
@@ -334,8 +344,9 @@ onClickOutside(smartTable, (e: PointerEvent) => {
   if (editEnabled && (isVirtualCol(activeCol) || activeCol.uidt === UITypes.JSON)) return
 
   // ignore unselecting if clicked inside or on the picker(Date, Time, DateTime, Year)
+  // or single/multi select options
   const activePickerEl = document.querySelector(
-    '.nc-picker-datetime.active,.nc-picker-date.active,.nc-picker-year.active,.nc-picker-time.active',
+    '.nc-picker-datetime.active,.nc-dropdown-single-select-cell.active,.nc-dropdown-multi-select-cell.active,.nc-picker-date.active,.nc-picker-year.active,.nc-picker-time.active',
   )
   if (e.target && activePickerEl && (activePickerEl === e.target || activePickerEl?.contains(e.target as Element))) return
 
diff --git a/packages/nc-gui/composables/useMultiSelect/index.ts b/packages/nc-gui/composables/useMultiSelect/index.ts
index 23f634af3a..f089a9aac9 100644
--- a/packages/nc-gui/composables/useMultiSelect/index.ts
+++ b/packages/nc-gui/composables/useMultiSelect/index.ts
@@ -18,6 +18,7 @@ export function useMultiSelect(
   clearCell: Function,
   makeEditable: Function,
   scrollToActiveCell?: (row?: number | null, col?: number | null) => void,
+  keyEventHandler?: Function,
 ) {
   const { t } = useI18n()
 
@@ -126,6 +127,11 @@ export function useMultiSelect(
   })
 
   const onKeyDown = async (e: KeyboardEvent) => {
+    // invoke the keyEventHandler if provided and return if it returns true
+    if (await keyEventHandler?.(e)) {
+      return
+    }
+
     if (
       !isNaN(selectedRows.startRow) &&
       !isNaN(selectedRows.startCol) &&