Merge pull request #5932 from nocodb/fix/form-select-field

fix: Form view single/multi select bug

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

@@ -27,6 +27,7 @@ import {
   useRoles,
   useSelectedCellKeyupListener,
   watch,
+  EditModeInj
 } from '#imports'
 import MdiCloseCircle from '~icons/mdi/close-circle'
 
@@ -45,7 +46,13 @@ const column = inject(ColumnInj)!
 
 const readOnly = inject(ReadonlyInj)!
 
-const active = inject(ActiveCellInj, ref(false))
+const isEditable = inject(EditModeInj, ref(false))
+
+const _active = inject(ActiveCellInj, ref(false))
+
+// use both ActiveCellInj or EditModeInj to determine the active state
+// since active will be false in case of form view
+const active = computed(() => _active.value || isEditable.value)
 
 const isPublic = inject(IsPublicInj, ref(false))
 
@@ -94,7 +101,7 @@ const isOptionMissing = computed(() => {
 
 const hasEditRoles = computed(() => hasRole('owner', true) || hasRole('creator', true) || hasRole('editor', true))
 
-const editAllowed = computed(() => (hasEditRoles.value || isForm.value) && active.value)
+const editAllowed = computed(() => (hasEditRoles.value || isForm.value) && (active.value))
 
 const vModel = computed({
   get: () => {

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

@@ -23,6 +23,7 @@ import {
   useRoles,
   useSelectedCellKeyupListener,
   watch,
+  EditModeInj
 } from '#imports'
 
 interface Props {
@@ -39,7 +40,13 @@ const column = inject(ColumnInj)!
 
 const readOnly = inject(ReadonlyInj)!
 
-const active = inject(ActiveCellInj, ref(false))
+const isEditable = inject(EditModeInj, ref(false))
+
+const _active = inject(ActiveCellInj, ref(false))
+
+// use both ActiveCellInj or EditModeInj to determine the active state
+// since active will be false in case of form view
+const active = computed(() => _active.value || isEditable.value)
 
 const aselect = ref<typeof AntSelect>()
 
@@ -89,7 +96,7 @@ const isOptionMissing = computed(() => {
 
 const hasEditRoles = computed(() => hasRole('owner', true) || hasRole('creator', true) || hasRole('editor', true))
 
-const editAllowed = computed(() => (hasEditRoles.value || isForm.value) && active.value)
+const editAllowed = computed(() => (hasEditRoles.value || isForm.value) && (active.value))
 
 const vModel = computed({
   get: () => tempSelectedOptState.value ?? modelValue,
@@ -250,7 +257,7 @@ const selectedOpt = computed(() => {
 
 <template>
   <div class="h-full w-full flex items-center nc-single-select" :class="{ 'read-only': readOnly }" @click="toggleMenu">
-    <div v-if="!active">
+    <div v-if="!(active || isEditable) ">
       <a-tag v-if="selectedOpt" class="rounded-tag" :color="selectedOpt.color">
         <span
           :style="{

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

@@ -1,17 +1,13 @@
 <script lang="ts" setup>
-import { CellClickHookInj, CurrentCellInj, createEventHook, ref } from '#imports'
+import { CurrentCellInj, ref } from '#imports'
 
 const el = ref()
 
-const cellClickHook = createEventHook()
-
-provide(CellClickHookInj, cellClickHook)
-
 provide(CurrentCellInj, el)
 </script>
 
 <template>
-  <div ref="el" class="select-none" @click="cellClickHook.trigger($event)">
+  <div ref="el" class="select-none">
     <slot />
   </div>
 </template>

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

@@ -28,13 +28,18 @@ export class SelectOptionCellPageObject extends BasePage {
     const selectCell = this.get({ index, columnHeader });
 
     // check if cell active
-    if (!(await selectCell.getAttribute('class')).includes('active')) {
+    if (
+      !(await selectCell.getAttribute('class')).includes('active') &&
+      (await selectCell.locator('.nc-selected-option').count()) === 0
+    ) {
       await selectCell.click();
     }
 
     await selectCell.click();
 
-    await this.rootPage.getByTestId(`select-option-${columnHeader}-${index}`).getByText(option).click();
+    if (index === -1)
+      await this.rootPage.getByTestId(`select-option-${columnHeader}-undefined`).getByText(option).click();
+    else await this.rootPage.getByTestId(`select-option-${columnHeader}-${index}`).getByText(option).click();
 
     if (multiSelect) await this.get({ index, columnHeader }).click();
 
@@ -72,12 +77,12 @@ export class SelectOptionCellPageObject extends BasePage {
   }
 
   async verify({
-    index,
+    index = 0,
     columnHeader,
     option,
     multiSelect,
   }: {
-    index: number;
+    index?: number;
     columnHeader: string;
     option: string;
     multiSelect?: boolean;
@@ -85,7 +90,10 @@ export class SelectOptionCellPageObject extends BasePage {
     if (multiSelect) {
       return await expect(this.cell.get({ index, columnHeader })).toContainText(option, { useInnerText: true });
     }
-    const text = await (await this.cell.get({ index, columnHeader }).locator('.ant-tag')).allInnerTexts();
+
+    const locator = await this.cell.get({ index, columnHeader }).locator('.ant-tag');
+    await locator.waitFor({ state: 'visible' });
+    const text = await locator.allInnerTexts();
     return expect(text).toContain(option);
   }
 
@@ -95,7 +103,15 @@ export class SelectOptionCellPageObject extends BasePage {
     ).toBeHidden();
   }
 
-  async verifyOptions({ index, columnHeader, options }: { index: number; columnHeader: string; options: string[] }) {
+  async verifyOptions({
+    index = 0,
+    columnHeader,
+    options,
+  }: {
+    index?: number;
+    columnHeader: string;
+    options: string[];
+  }) {
     const selectCell = this.get({ index, columnHeader });
 
     // check if cell active

tests/playwright/tests/db/viewForm.spec.ts

@@ -364,3 +364,98 @@ test.describe('Form view with LTAR', () => {
     });
   });
 });
+
+test.describe('Form view', () => {
+  let dashboard: DashboardPage;
+  let form: FormPage;
+  let context: any;
+
+  test.beforeEach(async ({ page }) => {
+    context = await setup({ page, isEmptyProject: true });
+    dashboard = new DashboardPage(page, context.project);
+    form = dashboard.form;
+  });
+
+  test('Select fields in form view', async () => {
+    api = new Api({
+      baseURL: `http://localhost:8080/`,
+      headers: {
+        'xc-auth': context.token,
+      },
+    });
+
+    const columns = [
+      {
+        column_name: 'Id',
+        title: 'Id',
+        uidt: UITypes.ID,
+      },
+      {
+        column_name: 'SingleSelect',
+        title: 'SingleSelect',
+        uidt: UITypes.SingleSelect,
+        dtxp: "'jan','feb','mar','apr','may','jun','jul','aug','sep','oct','nov','dec'",
+      },
+      {
+        column_name: 'MultiSelect',
+        title: 'MultiSelect',
+        uidt: UITypes.MultiSelect,
+        dtxp: "'jan','feb','mar','apr','may','jun','jul','aug','sep','oct','nov','dec'",
+      },
+    ];
+
+    const project = await api.project.read(context.project.id);
+    await api.base.tableCreate(context.project.id, project.bases?.[0].id, {
+      table_name: 'selectBased',
+      title: 'selectBased',
+      columns: columns,
+    });
+
+    await dashboard.rootPage.reload();
+
+    await dashboard.treeView.openTable({ title: 'selectBased' });
+    const url = dashboard.rootPage.url();
+
+    await dashboard.viewSidebar.createFormView({ title: 'NewForm' });
+    await dashboard.form.toolbar.clickShareView();
+    const formLink = await dashboard.form.toolbar.shareView.getShareLink();
+
+    await dashboard.rootPage.goto(formLink);
+
+    const sharedForm = new SharedFormPage(dashboard.rootPage);
+
+    // Click on single select options
+    await sharedForm.cell.selectOption.select({
+      index: -1,
+      columnHeader: 'SingleSelect',
+      option: 'jan',
+      multiSelect: false,
+    });
+
+    // Click on multi select options
+    const multiSelectParams = {
+      index: -1,
+      columnHeader: 'MultiSelect',
+      option: 'jan',
+      multiSelect: true,
+    };
+    await sharedForm.cell.selectOption.select({ ...multiSelectParams, option: 'jan' });
+    await sharedForm.cell.selectOption.select({ ...multiSelectParams, option: 'feb' });
+    await sharedForm.cell.selectOption.select({ ...multiSelectParams, option: 'mar' });
+
+    await sharedForm.submit();
+    await dashboard.rootPage.goto(url);
+    await dashboard.viewSidebar.openView({ title: 'selectBased' });
+
+    await dashboard.grid.cell.selectOption.verify({
+      columnHeader: 'SingleSelect',
+      option: 'jan',
+      multiSelect: false,
+    });
+
+    await dashboard.grid.cell.selectOption.verifyOptions({
+      columnHeader: 'MultiSelect',
+      options: ['jan', 'feb', 'mar'],
+    });
+  });
+});