Merge pull request #5271 from nocodb/5260-ui-bug-fixes-and-enhancement

UI enhancement and fixes

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

@@ -89,7 +89,7 @@ useSelectedCellKeyupListener(active, (e) => {
 
 <style scoped lang="scss">
 .nc-cell-hover-show {
-  opacity: 0;
+  opacity: 0.3;
   transition: 0.3s opacity;
 
   &:hover {

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

@@ -29,7 +29,7 @@ onMounted(() => {
     -->
     <text-clamp
       :key="`clamp-${key}-${props.value?.toString().length || 0}`"
-      class="w-full h-full break-all"
+      class="w-full h-full break-word"
       :text="`${props.value || ' '}`"
       :max-lines="props.lines"
     />

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

@@ -81,6 +81,7 @@ onMounted(() => {
     @keydown.delete.stop
     @selectstart.capture.stop
     @mousedown.stop
+    @contextmenu.stop
   />
 
   <span v-else-if="vModel === null && showNull" class="nc-null">NULL</span>

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

@@ -14,6 +14,8 @@ const { showNull } = useGlobal()
 
 const editEnabled = inject(EditModeInj)
 
+const rowHeight = inject(RowHeightInj)
+
 const readonly = inject(ReadonlyInj, ref(false))
 
 const vModel = useVModel(props, 'modelValue', emits)
@@ -42,5 +44,5 @@ const focus: VNodeRef = (el) => {
 
   <span v-else-if="vModel === null && showNull" class="nc-null">NULL</span>
 
-  <LazyCellClampedText v-else :value="vModel" :lines="1" />
+  <LazyCellClampedText v-else :value="vModel" :lines="rowHeight" />
 </template>

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

@@ -45,3 +45,8 @@ const focus: VNodeRef = (el) => (el as HTMLTextAreaElement)?.focus()
 
   <span v-else>{{ vModel }}</span>
 </template>
+<style>
+textarea:focus {
+  box-shadow: none;
+}
+</style>

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

@@ -139,6 +139,15 @@ const isNumericField = computed(() => {
     isDuration(column.value)
   )
 })
+
+// disable contexxtmenu event propagation when cell is in
+// editable state and typable (e.g. text area)
+// this is to prevent the custom grid view context menu from opening
+const onContextmenu = (e: MouseEvent) => {
+  if (props.editEnabled && isTypableInputColumn(column.value)) {
+    e.stopPropagation()
+  }
+}
 </script>
 
 <template>
@@ -151,6 +160,7 @@ const isNumericField = computed(() => {
     ]"
     @keydown.enter.exact="syncAndNavigate(NavigateDir.NEXT, $event)"
     @keydown.shift.enter.exact="syncAndNavigate(NavigateDir.PREV, $event)"
+    @contextmenu="onContextmenu"
   >
     <template v-if="column">
       <LazyCellTextArea v-if="isTextArea(column)" v-model="vModel" />

packages/nc-gui/components/smartsheet/expanded-form/Header.vue

@@ -101,17 +101,6 @@ const onConfirmDeleteRowClick = async () => {
     </h5>
 
     <div class="flex-1" />
-
-    <a-tooltip placement="bottom">
-      <template #title>
-        <div class="text-center w-full">{{ $t('general.reload') }}</div>
-      </template>
-      <mdi-reload
-        v-if="!isNew"
-        class="nc-icon-transition cursor-pointer select-none text-gray-500 mx-1 min-w-4"
-        @click="loadRow"
-      />
-    </a-tooltip>
     <a-tooltip placement="bottom">
       <template #title>
         <!-- todo: i18n -->
@@ -137,32 +126,6 @@ const onConfirmDeleteRowClick = async () => {
       />
     </a-tooltip>
 
-    <a-tooltip v-if="!isSqlView" placement="bottom">
-      <!-- Duplicate row -->
-      <template #title>
-        <div class="text-center w-full">{{ $t('activity.duplicateRow') }}</div>
-      </template>
-      <MdiContentCopy
-        v-if="isUIAllowed('xcDatatableEditable') && !isNew"
-        v-e="['c:row-expand:duplicate']"
-        class="nc-icon-transition cursor-pointer select-none nc-duplicate-row text-gray-500 mx-1 min-w-4"
-        @click="!isNew && emit('duplicateRow')"
-      />
-    </a-tooltip>
-
-    <a-tooltip v-if="!isSqlView" placement="bottom">
-      <!-- Delete row -->
-      <template #title>
-        <div class="text-center w-full">{{ $t('activity.deleteRow') }}</div>
-      </template>
-      <MdiDeleteOutline
-        v-if="isUIAllowed('xcDatatableEditable') && !isNew"
-        v-e="['c:row-expand:delete']"
-        class="nc-icon-transition cursor-pointer select-none nc-delete-row text-gray-500 mx-1 min-w-4"
-        @click="!isNew && onDeleteRowClick()"
-      />
-    </a-tooltip>
-
     <a-dropdown-button class="nc-expand-form-save-btn" type="primary" :disabled="!isUIAllowed('tableRowUpdate')" @click="save">
       <template #icon><MdiMenuDown /></template>
 
@@ -192,17 +155,39 @@ const onConfirmDeleteRowClick = async () => {
       </div>
     </a-dropdown-button>
 
-    <a-tooltip placement="bottom">
-      <!-- Close -->
-      <template #title>
-        <div class="text-center w-full">{{ $t('general.close') }}</div>
+    <a-dropdown>
+      <MdiDotsVertical class="nc-icon-transition" />
+      <template #overlay>
+        <a-menu>
+          <a-menu-item v-if="!isNew" @click="loadRow">
+            <div v-e="['c:row-expand:reload']" class="py-2 flex gap-2 items-center">
+              <mdi-reload class="nc-icon-transition cursor-pointer select-none text-gray-500 mx-1 min-w-4" />
+              {{ $t('general.reload') }}
+            </div>
+          </a-menu-item>
+          <a-menu-item v-if="isUIAllowed('xcDatatableEditable') && !isNew" @click="!isNew && emit('duplicateRow')">
+            <div v-e="['c:row-expand:duplicate']" class="py-2 flex gap-2 a">
+              <MdiContentCopy class="nc-icon-transition cursor-pointer select-none nc-duplicate-row text-gray-500 mx-1 min-w-4" />
+              {{ $t('activity.duplicateRow') }}
+            </div>
+          </a-menu-item>
+          <a-menu-item v-if="isUIAllowed('xcDatatableEditable') && !isNew" @click="!isNew && onDeleteRowClick()">
+            <div v-e="['c:row-expand:delete']" class="py-2 flex gap-2 items-center">
+              <MdiDeleteOutline class="nc-icon-transition cursor-pointer select-none nc-delete-row text-gray-500 mx-1 min-w-4" />
+              {{ $t('activity.deleteRow') }}
+            </div>
+          </a-menu-item>
+          <a-menu-item @click="emit('cancel')">
+            <div v-e="['c:row-expand:delete']" class="py-2 flex gap-2 items-center">
+              <MdiCloseCircleOutline
+                class="nc-icon-transition cursor-pointer select-none nc-delete-row text-gray-500 mx-1 min-w-4"
+              />
+              {{ $t('general.close') }}
+            </div>
+          </a-menu-item>
+        </a-menu>
       </template>
-      <MdiCloseCircleOutline
-        class="nc-icon-transition cursor-pointer select-none nc-close-form text-gray-500 mx-1 min-w-4"
-        @click="emit('cancel')"
-      />
-    </a-tooltip>
-
+    </a-dropdown>
     <a-modal v-model:visible="showDeleteRowModal" title="Delete row?" @ok="onConfirmDeleteRowClick">
       <p>Are you sure you want to delete this row?</p>
     </a-modal>

packages/nc-gui/components/smartsheet/expanded-form/index.vue

@@ -141,8 +141,22 @@ const onDuplicateRow = () => {
 }
 
 const onNext = async () => {
-  await save()
-  emits('next')
+  if (changedColumns.value.size > 0) {
+    await Modal.confirm({
+      title: 'Do you want to save the changes?',
+      okText: 'Save',
+      cancelText: 'Discard',
+      onOk: async () => {
+        await save()
+        emits('next')
+      },
+      onCancel: () => {
+        emits('next')
+      },
+    })
+  } else {
+    emits('next')
+  }
 }
 
 const reloadParentRowHook = inject(ReloadRowDataHookInj, createEventHook())
@@ -196,7 +210,7 @@ useActiveKeyupListener(
       emits('prev')
     } else if (e.key === 'ArrowRight') {
       e.stopPropagation()
-      emits('next')
+      onNext()
     }
     // on alt + s save record
     else if (e.code === 'KeyS') {
@@ -319,7 +333,8 @@ export default {
                 :ref="i ? null : (el) => (cellWrapperEl = el)"
                 class="!bg-white rounded px-1 min-h-[35px] flex items-center mt-2 relative"
               >
-                <LazySmartsheetVirtualCell v-if="isVirtualCol(col)" v-model="row.row[col.title]" :row="row" :column="col" />
+                <LazySmartsheetVirtualCell v-if="isVirtualCol(col)" v-model="row.row[col.title]" :row="row"
+                                           :column="col" />
 
                 <LazySmartsheetCell
                   v-else

packages/nc-gui/components/smartsheet/toolbar/ColumnFilter.vue

@@ -198,10 +198,15 @@ defineExpose({
 
 <template>
   <div
-    class="p-4 menu-filter-dropdown bg-gray-50 !border mt-4"
-    :class="{ 'shadow min-w-[430px] max-h-[max(80vh,500px)] overflow-auto': !nested, 'border-1 w-full': nested }"
+    class="p-4 menu-filter-dropdown bg-gray-50 !border"
+    :class="{  'min-w-[430px]': filters.length, 'shadow max-h-[max(80vh,500px)] overflow-auto': !nested, 'border-1 w-full': nested }"
   >
-    <div v-if="filters && filters.length" class="nc-filter-grid mb-2" @click.stop>
+    <div
+      v-if="filters && filters.length"
+      class="nc-filter-grid mb-2"
+      :class="{ 'max-h-420px overflow-y-auto': !nested }"
+      @click.stop
+    >
       <template v-for="(filter, i) in filters" :key="i">
         <template v-if="filter.status !== 'delete'">
           <template v-if="filter.is_group">
@@ -230,7 +235,7 @@ defineExpose({
               </a-select>
             </div>
             <span class="col-span-3" />
-            <div class="col-span-5">
+            <div class="col-span-6">
               <LazySmartsheetToolbarColumnFilter
                 v-if="filter.id || filter.children"
                 :key="filter.id ?? i"

packages/nc-gui/components/smartsheet/toolbar/SortListMenu.vue

@@ -1,4 +1,5 @@
 <script setup lang="ts">
+import { nextTick } from '@vue/runtime-core'
 import type { ColumnType } from 'nocodb-sdk'
 import {
   ActiveViewInj,
@@ -22,7 +23,17 @@ const reloadDataHook = inject(ReloadViewDataHookInj)
 
 const { eventBus } = useSmartsheetStoreOrThrow()
 
-const { sorts, saveOrUpdate, loadSorts, addSort, deleteSort } = useViewSorts(view, () => reloadDataHook?.trigger())
+const { sorts, saveOrUpdate, loadSorts, addSort: _addSort, deleteSort } = useViewSorts(view, () => reloadDataHook?.trigger())
+
+const removeIcon = ref<HTMLElement>()
+
+const addSort = () => {
+  _addSort()
+  nextTick(() => {
+    console.log(removeIcon.value)
+    removeIcon.value?.[removeIcon.value?.length - 1]?.$el?.scrollIntoView()
+  })
+}
 
 eventBus.on((event) => {
   if (event === SmartsheetStoreEvents.SORT_RELOAD) {
@@ -75,12 +86,18 @@ useMenuCloseOnEsc(open)
     </div>
     <template #overlay>
       <div
-        class="bg-gray-50 p-6 shadow-lg menu-filter-dropdown min-w-[400px] max-h-[max(80vh,500px)] overflow-auto !border"
+        :class="{ ' min-w-[400px]': sorts.length }"
+        class="bg-gray-50 p-6 shadow-lg menu-filter-dropdown max-h-[max(80vh,500px)] overflow-auto !border"
         data-testid="nc-sorts-menu"
       >
-        <div v-if="sorts?.length" class="sort-grid mb-2" @click.stop>
+        <div v-if="sorts?.length" class="sort-grid mb-2 max-h-420px overflow-y-auto" @click.stop>
           <template v-for="(sort, i) of sorts" :key="i">
-            <MdiCloseBox class="nc-sort-item-remove-btn text-grey self-center" small @click.stop="deleteSort(sort, i)" />
+            <MdiCloseBox
+              ref="removeIcon"
+              class="nc-sort-item-remove-btn text-grey self-center"
+              small
+              @click.stop="deleteSort(sort, i)"
+            />
 
             <LazySmartsheetToolbarFieldListAutoCompleteDropdown
               v-model="sort.fk_column_id"
@@ -92,6 +109,7 @@ useMenuCloseOnEsc(open)
             />
 
             <a-select
+              ref=""
               v-model:value="sort.direction"
               class="shrink grow-0 nc-sort-dir-select !text-xs"
               :label="$t('labels.operation')"

packages/nc-gui/components/webhook/Editor.vue

@@ -651,6 +651,7 @@ onMounted(loadPluginList)
             </a-checkbox>
 
             <LazySmartsheetToolbarColumnFilter
+              class="mt-4"
               v-if="hook.condition"
               ref="filterRef"
               :auto-save="false"

packages/nc-gui/composables/useExpandedFormStore.ts

@@ -30,13 +30,13 @@ const [useProvideExpandedFormStore, useExpandedFormStore] = useInjectionState((m
 
   const { t } = useI18n()
 
-  const commentsOnly = ref(false)
+  const commentsOnly = ref(true)
 
   const commentsAndLogs = ref<any[]>([])
 
   const comment = ref('')
 
-  const commentsDrawer = ref(false)
+  const commentsDrawer = ref(true)
 
   const saveRowAndStay = ref(0)
 

tests/playwright/pages/Dashboard/ExpandedForm/index.ts

@@ -24,16 +24,25 @@ export class ExpandedFormPage extends BasePage {
     return this.dashboard.get().locator(`.nc-drawer-expanded-form`);
   }
 
-  async clickDuplicateRow() {
-    await this.duplicateRowButton.click();
+  async click3DotsMenu(menuItem: string) {
+    await this.get().locator('.nc-icon-transition.ant-dropdown-trigger').last().click();
+
+    // add delay; wait for the menu to appear
+    await this.rootPage.waitForTimeout(500);
 
+    const popUpMenu = await this.rootPage.locator('.ant-dropdown');
+    await popUpMenu.locator(`.ant-dropdown-menu-item:has-text("${menuItem}")`).click();
+  }
+
+  async clickDuplicateRow() {
+    await this.click3DotsMenu('Duplicate Row');
     // wait for loader to disappear
     // await this.dashboard.waitForLoaderToDisappear();
     await this.rootPage.waitForTimeout(2000);
   }
 
   async clickDeleteRow() {
-    await this.deleteRowButton.click();
+    await this.click3DotsMenu('Delete Row');
     await this.rootPage.locator('.ant-btn-primary:has-text("OK")').click();
   }
 
@@ -142,7 +151,7 @@ export class ExpandedFormPage extends BasePage {
   }
 
   async close() {
-    await this.get().locator('.nc-close-form').last().click();
+    await this.click3DotsMenu('Close');
   }
 
   async openChildCard(param: { column: string; title: string }) {

tests/playwright/pages/Dashboard/Grid/index.ts

@@ -283,6 +283,9 @@ export class GridPage extends BasePage {
     });
     await expect(await cell.locator('input')).toBeVisible();
 
+    // press escape to exit edit mode
+    await cell.press('Escape');
+
     // right click menu
     await this.get().locator(`td[data-testid="cell-${columnHeader}-0"]`).click({
       button: 'right',

tests/playwright/pages/Dashboard/common/Cell/index.ts

@@ -289,8 +289,11 @@ export class CellPageObject extends BasePage {
     // editable cell
     await cell.dblclick();
     await expect(await cell.locator(`input`)).toHaveCount(param.role === 'creator' || param.role === 'editor' ? 1 : 0);
-    // right click context menu
-    await cell.click({ button: 'right' });
+
+    // press escape to close the input
+    await cell.press('Escape');
+
+    await cell.click({ button: 'right', clickCount: 1 });
     await expect(await this.rootPage.locator(`.nc-dropdown-grid-context-menu:visible`)).toHaveCount(
       param.role === 'creator' || param.role === 'editor' ? 1 : 0
     );