Merge pull request #7348 from nocodb/nc-fix/select-type-related-fields-ui-ux

Fix expand button overlap issue on Select-Type related fields & NULL display in cell uniform issue
10 months ago · 750bdd89f6
17 changed files with 98 additions and 37 deletions
--- a/packages/nc-gui/components/cell/DatePicker.vue
+++ b/packages/nc-gui/components/cell/DatePicker.vue
@ -104,7 +104,7 @@ const placeholder = computed(() => {
  if (isEditColumn.value && (modelValue === '' || modelValue === null)) {
    return t('labels.optional')
  } else if (modelValue === null && showNull.value) {
-    return t('general.null')
+    return t('general.null').toUpperCase()
  } else if (isDateInvalid.value) {
    return t('msg.invalidDate')
  } else {
--- a/packages/nc-gui/components/cell/DateTimePicker.vue
+++ b/packages/nc-gui/components/cell/DateTimePicker.vue
@ -157,7 +157,7 @@ const placeholder = computed(() => {
  if (isEditColumn.value && (modelValue === '' || modelValue === null)) {
    return t('labels.optional')
  } else if (modelValue === null && showNull.value) {
-    return t('general.null')
+    return t('general.null').toUpperCase()
  } else if (isDateInvalid.value) {
    return t('msg.invalidDate')
  } else {
--- a/packages/nc-gui/components/cell/Decimal.vue
+++ b/packages/nc-gui/components/cell/Decimal.vue
@ -112,7 +112,7 @@ watch(isExpandedFormOpen, () => {
    @selectstart.capture.stop
    @mousedown.stop
  />
-  <span v-else-if="vModel === null && showNull" class="nc-null capitalize">{{ $t('general.null') }}</span>
+  <span v-else-if="vModel === null && showNull" class="nc-null uppercase">{{ $t('general.null') }}</span>
  <span v-else class="text-sm">{{ displayValue }}</span>
 </template>

--- a/packages/nc-gui/components/cell/Duration.vue
+++ b/packages/nc-gui/components/cell/Duration.vue
@ -111,7 +111,7 @@ const focus: VNodeRef = (el) =>
      @mousedown.stop
    />

-    <span v-else-if="modelValue === null && showNull" class="nc-null capitalize">{{ $t('general.null') }}</span>
+    <span v-else-if="modelValue === null && showNull" class="nc-null uppercase">{{ $t('general.null') }}</span>

    <span v-else> {{ localState }}</span>

--- a/packages/nc-gui/components/cell/MultiSelect.vue
+++ b/packages/nc-gui/components/cell/MultiSelect.vue
@ -567,7 +567,7 @@ const onFocus = () => {
 }

 :deep(.ant-select-selector) {
-  @apply !px-0;
+  @apply !pl-0;
 }

 :deep(.ant-select-selection-search-input) {
--- a/packages/nc-gui/components/cell/Percent.vue
+++ b/packages/nc-gui/components/cell/Percent.vue
@ -143,7 +143,7 @@ const onTabPress = (e: KeyboardEvent) => {
      @selectstart.capture.stop
      @mousedown.stop
    />
-    <span v-else-if="vModel === null && showNull" class="nc-null capitalize">{{ $t('general.null') }}</span>
+    <span v-else-if="vModel === null && showNull" class="nc-null uppercase">{{ $t('general.null') }}</span>
    <div v-else-if="percentMeta.is_progress === true && vModel !== null && vModel !== undefined" class="px-2">
      <a-progress
        :percent="Number(parseFloat(vModel.toString()).toFixed(2))"
--- a/packages/nc-gui/components/cell/SingleSelect.vue
+++ b/packages/nc-gui/components/cell/SingleSelect.vue
@ -322,7 +322,8 @@ const onFocus = () => {
      :disabled="readOnly || !editAllowed"
      :show-search="!isMobileMode && isOpen && active"
      :show-arrow="hasEditRoles && !readOnly && active && (vModel === null || vModel === undefined)"
-      :dropdown-class-name="`nc-dropdown-single-select-cell ${isOpen && active ? 'active' : ''}`"
+      :dropdown-class-name="`nc-dropdown-single-select-cell !min-w-200px ${isOpen && active ? 'active' : ''}`"
+      :dropdown-match-select-width="true"
      @select="onSelect"
      @keydown="onKeydown($event)"
      @search="search"
@ -399,7 +400,12 @@ const onFocus = () => {
 }

 :deep(.ant-select-selector) {
-  @apply !px-0;
+  @apply !pl-0 !pr-4;
+}
+
+:deep(.ant-select-selector .ant-select-selection-item) {
+  @apply flex items-center;
+  text-overflow: clip;
 }

 :deep(.ant-select-selection-search-input) {
--- a/packages/nc-gui/components/cell/TimePicker.vue
+++ b/packages/nc-gui/components/cell/TimePicker.vue
@ -96,7 +96,7 @@ const placeholder = computed(() => {
  if (isEditColumn.value && (modelValue === '' || modelValue === null)) {
    return t('labels.optional')
  } else if (modelValue === null && showNull.value) {
-    return t('general.null')
+    return t('general.null').toUpperCase()
  } else if (isTimeInvalid.value) {
    return t('msg.invalidTime')
  } else {
--- a/packages/nc-gui/components/cell/Url.vue
+++ b/packages/nc-gui/components/cell/Url.vue
@ -108,7 +108,7 @@ watch(
      @mousedown.stop
    />

-    <span v-else-if="vModel === null && showNull" class="nc-null uppercase"> $t('general.null')</span>
+    <span v-else-if="vModel === null && showNull" class="nc-null uppercase"> {{ $t('general.null') }}</span>

    <nuxt-link
      v-else-if="isValid && !cellUrlOptions?.overlay"
--- a/packages/nc-gui/components/cell/User.vue
+++ b/packages/nc-gui/components/cell/User.vue
@ -280,7 +280,7 @@ const filterOption = (input: string, option: any) => {
      }"
    >
      <template v-for="selectedOpt of vModel" :key="selectedOpt.value">
-        <a-tag class="rounded-tag" color="'#ccc'">
+        <a-tag class="rounded-tag max-w-full" color="'#ccc'">
          <span
            :style="{
              'color': tinycolor.isReadable('#ccc' || '#ccc', '#fff', { level: 'AA', size: 'large' })
@ -290,7 +290,21 @@ const filterOption = (input: string, option: any) => {
            }"
            :class="{ 'text-sm': isKanban }"
          >
-            {{ selectedOpt.label }}
+            <NcTooltip class="truncate max-w-full" show-on-truncate-only>
+              <template #title>
+                {{ selectedOpt.label }}
+              </template>
+              <span
+                class="text-ellipsis overflow-hidden"
+                :style="{
+                  wordBreak: 'keep-all',
+                  whiteSpace: 'nowrap',
+                  display: 'inline',
+                }"
+              >
+                {{ selectedOpt.label }}
+              </span>
+            </NcTooltip>
          </span>
        </a-tag>
      </template>
@ -310,7 +324,7 @@ const filterOption = (input: string, option: any) => {
      :open="isOpen && editAllowed"
      :disabled="readOnly || !editAllowed"
      :class="{ 'caret-transparent': !hasEditRoles }"
-      :dropdown-class-name="`nc-dropdown-user-select-cell ${isOpen ? 'active' : ''}`"
+      :dropdown-class-name="`nc-dropdown-user-select-cell !min-w-200px ${isOpen ? 'active' : ''}`"
      :filter-option="filterOption"
      @search="search"
      @keydown.stop
@ -326,7 +340,7 @@ const filterOption = (input: string, option: any) => {
          :class="`nc-select-option-${column.title}-${op.email}`"
          @click.stop
        >
-          <a-tag class="rounded-tag !pl-0" color="'#ccc'">
+          <a-tag class="rounded-tag max-w-full !pl-0" color="'#ccc'">
            <span
              :style="{
                'color': tinycolor.isReadable('#ccc' || '#ccc', '#fff', { level: 'AA', size: 'large' })
@ -338,9 +352,21 @@ const filterOption = (input: string, option: any) => {
              :class="{ 'text-sm': isKanban }"
            >
              <GeneralUserIcon size="medium" :email="op.email" />
-              <span>
-                {{ op.display_name?.length ? op.display_name : op.email }}
-              </span>
+              <NcTooltip class="truncate max-w-full" show-on-truncate-only>
+                <template #title>
+                  {{ op.display_name?.length ? op.display_name : op.email }}
+                </template>
+                <span
+                  class="text-ellipsis overflow-hidden"
+                  :style="{
+                    wordBreak: 'keep-all',
+                    whiteSpace: 'nowrap',
+                    display: 'inline',
+                  }"
+                >
+                  {{ op.display_name?.length ? op.display_name : op.email }}
+                </span>
+              </NcTooltip>
            </span>
          </a-tag>
        </a-select-option>
@ -437,7 +463,7 @@ const filterOption = (input: string, option: any) => {
 }

 :deep(.ant-select-selector) {
-  @apply !px-0;
+  @apply !pl-0;
 }

 :deep(.ant-select-selection-search-input) {
--- a/packages/nc-gui/components/cell/YearPicker.vue
+++ b/packages/nc-gui/components/cell/YearPicker.vue
@ -83,7 +83,7 @@ const placeholder = computed(() => {
  if (isEditColumn.value && (modelValue === '' || modelValue === null)) {
    return t('labels.optional')
  } else if (modelValue === null && showNull.value) {
-    return t('general.null')
+    return t('general.null').toUpperCase()
  } else if (isYearInvalid.value) {
    return t('msg.invalidTime')
  } else {
--- a/tests/playwright/pages/Dashboard/Grid/Column/UserOptionColumn.ts
+++ b/tests/playwright/pages/Dashboard/Grid/Column/UserOptionColumn.ts
@ -51,6 +51,8 @@ export class UserOptionColumnPageObject extends BasePage {
    const selector = this.column.get().locator('.nc-user-select >> .ant-select-selector');
    await selector.click();

+    await this.rootPage.locator('.nc-dropdown-user-select-cell').waitFor({ state: 'visible' });
+
    if (multiSelect) {
      const optionsToSelect = Array.isArray(option) ? option : [option];

@ -60,11 +62,12 @@ export class UserOptionColumnPageObject extends BasePage {

      // Press `Escape` to close the dropdown
      await this.rootPage.keyboard.press('Escape');
-      await this.rootPage.locator('.nc-dropdown-user-select-cell').waitFor({ state: 'hidden' });
    } else if (!Array.isArray(option)) {
      await this.selectOption({ option });
    }

+    await this.rootPage.locator('.nc-dropdown-user-select-cell').waitFor({ state: 'hidden' });
+
    await this.column.save({ isUpdated: true });
  }

@ -91,6 +94,8 @@ export class UserOptionColumnPageObject extends BasePage {

    await this.column.get().locator('.nc-cell-user > .nc-user-select').click();

+    await this.rootPage.locator('.nc-dropdown-user-select-cell').waitFor({ state: 'visible' });
+
    expect(await this.rootPage.getByTestId(`select-option-${columnTitle}-undefined`).count()).toEqual(totalCount);
    await this.column.get().locator('.nc-cell-user').click();

--- a/tests/playwright/pages/Dashboard/Grid/Group.ts
+++ b/tests/playwright/pages/Dashboard/Grid/Group.ts
@ -74,6 +74,7 @@ export class GroupPageObject extends BasePage {
    value: string;
  }) {
    const gridWrapper = this.get({ indexMap });
+    await gridWrapper.scrollIntoViewIfNeeded();
    await gridWrapper
      .locator(`.nc-group-table .nc-grid-row:nth-child(${rowIndex + 1}) [data-title="${columnHeader}"]`)
      .scrollIntoViewIfNeeded();
@ -109,6 +110,14 @@ export class GroupPageObject extends BasePage {
    await this.get({ indexMap }).locator('.nc-grid-add-new-row').click();

    const rowCount = index + 1;
+
+    const isRowSaving = this.get({ indexMap }).getByTestId(`row-save-spinner-${rowCount}`);
+    // if required field is present then isRowSaving will be hidden (not present in DOM)
+    await isRowSaving?.waitFor({ state: 'hidden' });
+
+    // fallback
+    await this.rootPage.waitForTimeout(400);
+
    await expect(this.get({ indexMap }).locator('.nc-grid-row')).toHaveCount(rowCount);

    await this._fillRow({ indexMap, index, columnHeader, value: rowValue });
--- a/tests/playwright/pages/Dashboard/Grid/index.ts
+++ b/tests/playwright/pages/Dashboard/Grid/index.ts
@ -123,10 +123,15 @@ export class GridPage extends BasePage {

    await this.get().locator('.nc-grid-add-new-cell').click();

-    // wait for insert row response
+    const rowCount = index + 1;
+
+    const isRowSaving = this.rootPage.getByTestId(`row-save-spinner-${rowCount}`);
+    // if required field is present then isRowSaving will be hidden (not present in DOM)
+    await isRowSaving?.waitFor({ state: 'hidden' });
+
+    // fallback
    await this.rootPage.waitForTimeout(400);

-    const rowCount = index + 1;
    await expect(this.get().locator('.nc-grid-row')).toHaveCount(rowCount);

    await this._fillRow({ index, columnHeader, value: rowValue });
--- a/tests/playwright/pages/Dashboard/common/Cell/SelectOptionCell.ts
+++ b/tests/playwright/pages/Dashboard/common/Cell/SelectOptionCell.ts
@ -39,22 +39,29 @@ export class SelectOptionCellPageObject extends BasePage {

    await selectCell.click();

+    if (multiSelect) {
+      await this.rootPage.locator('.nc-dropdown-multi-select-cell').waitFor({ state: 'visible' });
+    } else {
+      await this.rootPage.locator('.nc-dropdown-single-select-cell').waitFor({ state: 'visible' });
+    }
+
    if (index === -1) {
      const selectOption = this.rootPage.getByTestId(`select-option-${columnHeader}-undefined`).getByText(option);
-      await selectOption.waitFor({ state: 'visible' });
+      await selectOption.scrollIntoViewIfNeeded();
      await selectOption.click();
    } else {
      const selectOption = this.rootPage.getByTestId(`select-option-${columnHeader}-${index}`).getByText(option);
-      await selectOption.waitFor({ state: 'visible' });
+      await selectOption.scrollIntoViewIfNeeded();
      await selectOption.click();
    }

-    if (multiSelect) await this.get({ index, columnHeader }).click();
-
-    await this.rootPage
-      .getByTestId(`select-option-${columnHeader}-${index}`)
-      .getByText(option)
-      .waitFor({ state: 'hidden' });
+    if (multiSelect) {
+      // Press `Escape` to close the dropdown
+      await this.rootPage.keyboard.press('Escape');
+      await this.rootPage.locator('.nc-dropdown-multi-select-cell').waitFor({ state: 'hidden' });
+    } else {
+      await this.rootPage.locator('.nc-dropdown-single-select-cell').waitFor({ state: 'hidden' });
+    }
  }

  async clear({ index, columnHeader, multiSelect }: { index: number; columnHeader: string; multiSelect?: boolean }) {
--- a/tests/playwright/pages/Dashboard/common/Cell/UserOptionCell.ts
+++ b/tests/playwright/pages/Dashboard/common/Cell/UserOptionCell.ts
@ -37,22 +37,24 @@ export class UserOptionCellPageObject extends BasePage {

    await selectCell.click();

+    await this.rootPage.locator('.nc-dropdown-user-select-cell').waitFor({ state: 'visible' });
+
    if (index === -1) {
      const selectOption = this.rootPage.getByTestId(`select-option-${columnHeader}-undefined`).getByText(option);
-      await selectOption.waitFor({ state: 'visible' });
+      await selectOption.scrollIntoViewIfNeeded();
      await selectOption.click();
    } else {
      const selectOption = this.rootPage.getByTestId(`select-option-${columnHeader}-${index}`).getByText(option);
-      await selectOption.waitFor({ state: 'visible' });
+      await selectOption.scrollIntoViewIfNeeded();
      await selectOption.click();
    }

-    if (multiSelect) await this.get({ index, columnHeader }).click();
+    if (multiSelect) {
+      // Press `Escape` to close the dropdown
+      await this.rootPage.keyboard.press('Escape');
+    }

-    await this.rootPage
-      .getByTestId(`select-option-${columnHeader}-${index}`)
-      .getByText(option)
-      .waitFor({ state: 'hidden' });
+    await this.rootPage.locator('.nc-dropdown-user-select-cell').waitFor({ state: 'hidden' });
  }

  async clear({ index, columnHeader, multiSelect }: { index: number; columnHeader: string; multiSelect?: boolean }) {
--- a/tests/playwright/tests/db/views/viewForm.spec.ts
+++ b/tests/playwright/tests/db/views/viewForm.spec.ts
@ -478,6 +478,7 @@ test.describe('Form view', () => {
      columnHeader: 'SingleSelect',
      option: 'jan',
      multiSelect: false,
+      ignoreDblClick: true,
    });

    // Click on multi select options