From e94ebe9f4ce17f0c389f6f30e4f21e2dda1e1183 Mon Sep 17 00:00:00 2001 From: Ekaterina Balakina Date: Thu, 15 Dec 2022 15:53:09 +0100 Subject: [PATCH 1/3] fix multiselect --- .../nc-gui/components/smartsheet/Cell.vue | 1 + .../nc-gui/components/smartsheet/Grid.vue | 77 ++++---- .../composables/useMultiSelect/cellRange.ts | 28 ++- .../composables/useMultiSelect/index.ts | 167 ++++++++++-------- packages/nc-gui/lib/types.ts | 2 + .../playwright/pages/Dashboard/Grid/index.ts | 32 +++- .../pages/Dashboard/common/Cell/index.ts | 41 ++--- tests/playwright/tests/cellSelection.spec.ts | 111 ++++++++++++ 8 files changed, 311 insertions(+), 148 deletions(-) create mode 100644 tests/playwright/tests/cellSelection.spec.ts diff --git a/packages/nc-gui/components/smartsheet/Cell.vue b/packages/nc-gui/components/smartsheet/Cell.vue index a64d08d1b4..9061933bb0 100644 --- a/packages/nc-gui/components/smartsheet/Cell.vue +++ b/packages/nc-gui/components/smartsheet/Cell.vue @@ -163,6 +163,7 @@ const syncAndNavigate = (dir: NavigateDir, e: KeyboardEvent) => { v-if="(isLocked || (isPublic && readOnly && !isForm) || isSystemColumn(column)) && !isAttachment(column)" class="nc-locked-overlay" @click.stop.prevent + @dblclick.stop.prevent /> diff --git a/packages/nc-gui/components/smartsheet/Grid.vue b/packages/nc-gui/components/smartsheet/Grid.vue index 0bcddde8d4..4dc08c0990 100644 --- a/packages/nc-gui/components/smartsheet/Grid.vue +++ b/packages/nc-gui/components/smartsheet/Grid.vue @@ -172,7 +172,7 @@ const getContainerScrollForElement = ( return scroll } -const { selectCell, startSelectRange, endSelectRange, clearSelectedRange, copyValue, isCellSelected, selectedCell } = +const { isCellSelected, activeCell, handleMouseDown, handleMouseOver, handleCellClick, clearSelectedRange, copyValue } = useMultiSelect( meta, fields, @@ -201,9 +201,10 @@ const { selectCell, startSelectRange, endSelectRange, clearSelectedRange, copyVa const cmdOrCtrl = isMac() ? e.metaKey : e.ctrlKey const altOrOptionKey = e.altKey if (e.key === ' ') { - if (selectedCell.row !== null && !editEnabled) { + if (activeCell.row != null && !editEnabled) { e.preventDefault() - const row = data.value[selectedCell.row] + clearSelectedRange() + const row = data.value[activeCell.row] expandForm(row) return true } @@ -227,29 +228,34 @@ const { selectCell, startSelectRange, endSelectRange, clearSelectedRange, copyVa switch (e.key) { case 'ArrowUp': e.preventDefault() - selectedCell.row = 0 - selectedCell.col = selectedCell.col ?? 0 + clearSelectedRange() + $e('c:shortcut', { key: 'CTRL + ArrowUp' }) + activeCell.row = 0 + activeCell.col = activeCell.col ?? 0 scrollToCell?.() editEnabled = false return true case 'ArrowDown': e.preventDefault() - selectedCell.row = data.value.length - 1 - selectedCell.col = selectedCell.col ?? 0 + clearSelectedRange() + activeCell.row = data.value.length - 1 + activeCell.col = activeCell.col ?? 0 scrollToCell?.() editEnabled = false return true case 'ArrowRight': e.preventDefault() - selectedCell.row = selectedCell.row ?? 0 - selectedCell.col = fields.value?.length - 1 + clearSelectedRange() + activeCell.row = activeCell.row ?? 0 + activeCell.col = fields.value?.length - 1 scrollToCell?.() editEnabled = false return true case 'ArrowLeft': e.preventDefault() - selectedCell.row = selectedCell.row ?? 0 - selectedCell.col = 0 + clearSelectedRange() + activeCell.row = activeCell.row ?? 0 + activeCell.col = 0 scrollToCell?.() editEnabled = false return true @@ -279,7 +285,7 @@ const { selectCell, startSelectRange, endSelectRange, clearSelectedRange, copyVa }, async (ctx: { row: number; col?: number; updatedColumnTitle?: string }) => { const rowObj = data.value[ctx.row] - const columnObj = ctx.col !== null && ctx.col !== undefined ? fields.value[ctx.col] : null + const columnObj = ctx.col !== undefined ? fields.value[ctx.col] : null if (!ctx.updatedColumnTitle && isVirtualCol(columnObj)) { return @@ -291,10 +297,10 @@ const { selectCell, startSelectRange, endSelectRange, clearSelectedRange, copyVa ) function scrollToCell(row?: number | null, col?: number | null) { - row = row ?? selectedCell.row - col = col ?? selectedCell.col + row = row ?? activeCell.row + col = col ?? activeCell.col - if (row !== undefined && col !== undefined && row !== null && col !== null) { + if (row !== null && col !== null) { // get active cell const rows = tbodyEl.value?.querySelectorAll('tr') const cols = rows?.[row].querySelectorAll('td') @@ -455,13 +461,14 @@ useEventListener(document, 'keyup', async (e: KeyboardEvent) => { /** On clicking outside of table reset active cell */ const smartTable = ref(null) + onClickOutside(smartTable, (e) => { // do nothing if context menu was open if (contextMenu.value) return - clearSelectedRange() - if (selectedCell.col === null) return - const activeCol = fields.value[selectedCell.col] + if (activeCell.row === null || activeCell.col === null) return + + const activeCol = fields.value[activeCell.col] if (editEnabled && (isVirtualCol(activeCol) || activeCol.uidt === UITypes.JSON)) return @@ -482,25 +489,29 @@ onClickOutside(smartTable, (e) => { return } - selectedCell.row = null - selectedCell.col = null + clearSelectedRange() + activeCell.row = null + activeCell.col = null }) const onNavigate = (dir: NavigateDir) => { - if (selectedCell.row === null || selectedCell.col === null) return + if (activeCell.row === null || activeCell.col === null) return + editEnabled = false + clearSelectedRange() + switch (dir) { case NavigateDir.NEXT: - if (selectedCell.row < data.value.length - 1) { - selectedCell.row++ + if (activeCell.row < data.value.length - 1) { + activeCell.row++ } else { addEmptyRow() - selectedCell.row++ + activeCell.row++ } break case NavigateDir.PREV: - if (selectedCell.row > 0) { - selectedCell.row-- + if (activeCell.row > 0) { + activeCell.row-- } break } @@ -782,10 +793,10 @@ const closeAddColumnDropdown = () => { :data-key="rowIndex + columnObj.id" :data-col="columnObj.id" :data-title="columnObj.title" - @click="selectCell(rowIndex, colIndex)" + @mousedown="handleMouseDown($event, rowIndex, colIndex)" + @mouseover="handleMouseOver(rowIndex, colIndex)" + @click="handleCellClick($event, rowIndex, colIndex)" @dblclick="makeEditable(row, columnObj)" - @mousedown="startSelectRange($event, rowIndex, colIndex)" - @mouseover="endSelectRange(rowIndex, colIndex)" @contextmenu="showContextMenu($event, { row: rowIndex, col: colIndex })" >
@@ -793,7 +804,7 @@ const closeAddColumnDropdown = () => { v-if="isVirtualCol(columnObj)" v-model="row.row[columnObj.title]" :column="columnObj" - :active="selectedCell.col === colIndex && selectedCell.row === rowIndex" + :active="activeCell.col === colIndex && activeCell.row === rowIndex" :row="row" @navigate="onNavigate" /> @@ -803,10 +814,10 @@ const closeAddColumnDropdown = () => { v-model="row.row[columnObj.title]" :column="columnObj" :edit-enabled=" - !!hasEditPermission && !!editEnabled && selectedCell.col === colIndex && selectedCell.row === rowIndex + !!hasEditPermission && !!editEnabled && activeCell.col === colIndex && activeCell.row === rowIndex " :row-index="rowIndex" - :active="selectedCell.col === colIndex && selectedCell.row === rowIndex" + :active="activeCell.col === colIndex && activeCell.row === rowIndex" @update:edit-enabled="editEnabled = $event" @save="updateOrSaveRow(row, columnObj.title, state)" @navigate="onNavigate" @@ -872,7 +883,7 @@ const closeAddColumnDropdown = () => {
- +
{{ $t('general.copy') }} diff --git a/packages/nc-gui/composables/useMultiSelect/cellRange.ts b/packages/nc-gui/composables/useMultiSelect/cellRange.ts index c059cbfe12..7d0ccc0e41 100644 --- a/packages/nc-gui/composables/useMultiSelect/cellRange.ts +++ b/packages/nc-gui/composables/useMultiSelect/cellRange.ts @@ -1,6 +1,6 @@ export interface Cell { - row: number | null - col: number | null + row: number + col: number } export class CellRange { @@ -12,14 +12,22 @@ export class CellRange { this._end = end ?? this._start } - get start() { + isEmpty() { + return this._start == null || this._end == null + } + + isSingleCell() { + return !this.isEmpty() && this._start?.col === this._end?.col && this._start?.row === this._end?.row + } + + get start(): Cell { return { row: Math.min(this._start?.row ?? NaN, this._end?.row ?? NaN), col: Math.min(this._start?.col ?? NaN, this._end?.col ?? NaN), } } - get end() { + get end(): Cell { return { row: Math.max(this._start?.row ?? NaN, this._end?.row ?? NaN), col: Math.max(this._start?.col ?? NaN, this._end?.col ?? NaN), @@ -27,19 +35,11 @@ export class CellRange { } startRange(value: Cell) { - if (value == null) { - return - } - this._start = value this._end = value } endRange(value: Cell) { - if (value == null) { - return - } - this._end = value } @@ -47,8 +47,4 @@ export class CellRange { this._start = null this._end = null } - - isEmpty() { - return this._start == null || this._end == null - } } diff --git a/packages/nc-gui/composables/useMultiSelect/index.ts b/packages/nc-gui/composables/useMultiSelect/index.ts index a4c1024df5..104b7888b9 100644 --- a/packages/nc-gui/composables/useMultiSelect/index.ts +++ b/packages/nc-gui/composables/useMultiSelect/index.ts @@ -4,7 +4,7 @@ import { RelationTypes, UITypes, isVirtualCol } from 'nocodb-sdk' import type { Cell } from './cellRange' import { CellRange } from './cellRange' import convertCellData from './convertCellData' -import type { Row } from '~/lib' +import type { Nullable, Row } from '~/lib' import { copyTable, extractPkFromRow, @@ -22,11 +22,13 @@ import { useProject, } from '#imports' +const MAIN_MOUSE_PRESSED = 0 + /** * Utility to help with multi-selecting rows/cells in the smartsheet */ export function useMultiSelect( - _meta: MaybeRef, + _meta: MaybeRef, fields: MaybeRef, data: MaybeRef, _editEnabled: MaybeRef, @@ -51,15 +53,26 @@ export function useMultiSelect( const editEnabled = ref(_editEnabled) - const selectedCell = reactive({ row: null, col: null }) - const selectedRange = reactive(new CellRange()) let isMouseDown = $ref(false) + const selectedRange = reactive(new CellRange()) + + const activeCell = reactive>({ row: null, col: null }) + const columnLength = $computed(() => unref(fields)?.length) + function makeActive(row: number, col: number) { + if (activeCell.row === row && activeCell.col === col) { + return + } + + activeCell.row = row + activeCell.col = col + } + async function copyValue(ctx?: Cell) { try { - if (!selectedRange.isEmpty()) { + if (selectedRange.start !== null && selectedRange.end !== null && !selectedRange.isSingleCell()) { const cprows = unref(data).slice(selectedRange.start.row, selectedRange.end.row + 1) // slice the selected rows for copy const cpcols = unref(fields).slice(selectedRange.start.col, selectedRange.end.col + 1) // slice the selected cols for copy @@ -68,8 +81,8 @@ export function useMultiSelect( } else { // if copy was called with context (right click position) - copy value from context // else if there is just one selected cell, copy it's value - const cpRow = ctx?.row ?? selectedCell?.row - const cpCol = ctx?.col ?? selectedCell?.col + const cpRow = ctx?.row ?? activeCell.row + const cpCol = ctx?.col ?? activeCell.col if (cpRow != null && cpCol != null) { const rowObj = unref(data)[cpRow] @@ -93,29 +106,19 @@ export function useMultiSelect( } } - function selectCell(row: number, col: number) { - selectedRange.clear() - if (selectedCell.row === row && selectedCell.col === col) return - editEnabled.value = false - selectedCell.row = row - selectedCell.col = col - } - - function endSelectRange(row: number, col: number) { + function handleMouseOver(row: number, col: number) { if (!isMouseDown) { return } - selectedCell.row = null - selectedCell.col = null selectedRange.endRange({ row, col }) } function isCellSelected(row: number, col: number) { - if (selectedCell?.row === row && selectedCell?.col === col) { + if (activeCell.col === col && activeCell.row === row) { return true } - if (selectedRange.isEmpty()) { + if (selectedRange.start === null || selectedRange.end === null) { return false } @@ -127,46 +130,51 @@ export function useMultiSelect( ) } - function startSelectRange(event: MouseEvent, row: number, col: number) { + function handleMouseDown(event: MouseEvent, row: number, col: number) { // if there was a right click on selected range, don't restart the selection - const leftClickButton = 0 - if (event?.button !== leftClickButton && isCellSelected(row, col)) { + if (event?.button !== MAIN_MOUSE_PRESSED && isCellSelected(row, col)) { return } - if (unref(editEnabled)) { - event.preventDefault() - return - } + editEnabled.value = false + isMouseDown = true + selectedRange.startRange({ row, col }) + } + const handleCellClick = (event: MouseEvent, row: number, col: number) => { isMouseDown = true - selectedRange.clear() + editEnabled.value = false selectedRange.startRange({ row, col }) + selectedRange.endRange({ row, col }) + makeActive(row, col) + isMouseDown = false } - useEventListener(document, 'mouseup', (e) => { - // if the editEnabled is false prevent the mouseup event for not select text + const handleMouseUp = (event: MouseEvent) => { + // 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(() => { + makeActive(selectedRange.start.row, selectedRange.start.col) + }, 0) + + // if the editEnabled is false, prevent selecting text on mouseUp if (!unref(editEnabled)) { - e.preventDefault() + event.preventDefault() } isMouseDown = false - }) + } - const onKeyDown = async (e: KeyboardEvent) => { + const handleKeyDown = async (e: KeyboardEvent) => { // invoke the keyEventHandler if provided and return if it returns true if (await keyEventHandler?.(e)) { return true } - if (!selectedRange.isEmpty()) { - // In case the user press tabs or arrows keys - selectedCell.row = selectedRange.start.row - selectedCell.col = selectedRange.start.col + if (activeCell.row === null || activeCell.col === null) { + return } - if (selectedCell.row === null || selectedCell.col === null) return - /** on tab key press navigate through cells */ switch (e.key) { case 'Tab': @@ -174,21 +182,21 @@ export function useMultiSelect( selectedRange.clear() if (e.shiftKey) { - if (selectedCell.col > 0) { - selectedCell.col-- + if (activeCell.col > 0) { + activeCell.col-- editEnabled.value = false - } else if (selectedCell.row > 0) { - selectedCell.row-- - selectedCell.col = unref(columnLength) - 1 + } else if (activeCell.row > 0) { + activeCell.row-- + activeCell.col = unref(columnLength) - 1 editEnabled.value = false } } else { - if (selectedCell.col < unref(columnLength) - 1) { - selectedCell.col++ + if (activeCell.col < unref(columnLength) - 1) { + activeCell.col++ editEnabled.value = false - } else if (selectedCell.row < unref(data).length - 1) { - selectedCell.row++ - selectedCell.col = 0 + } else if (activeCell.row < unref(data).length - 1) { + activeCell.row++ + activeCell.col = 0 editEnabled.value = false } } @@ -198,63 +206,68 @@ export function useMultiSelect( case 'Enter': e.preventDefault() selectedRange.clear() - makeEditable(unref(data)[selectedCell.row], unref(fields)[selectedCell.col]) + + makeEditable(unref(data)[activeCell.row], unref(fields)[activeCell.col]) break /** on delete key press clear cell */ case 'Delete': e.preventDefault() selectedRange.clear() - await clearCell(selectedCell as { row: number; col: number }) + + await clearCell(activeCell as { row: number; col: number }) break /** on arrow key press navigate through cells */ case 'ArrowRight': e.preventDefault() selectedRange.clear() - if (selectedCell.col < unref(columnLength) - 1) { - selectedCell.col++ + + if (activeCell.col < unref(columnLength) - 1) { + activeCell.col++ scrollToActiveCell?.() editEnabled.value = false } break case 'ArrowLeft': - selectedRange.clear() e.preventDefault() - if (selectedCell.col > 0) { - selectedCell.col-- + selectedRange.clear() + + if (activeCell.col > 0) { + activeCell.col-- scrollToActiveCell?.() editEnabled.value = false } break case 'ArrowUp': - selectedRange.clear() e.preventDefault() - if (selectedCell.row > 0) { - selectedCell.row-- + selectedRange.clear() + + if (activeCell.row > 0) { + activeCell.row-- scrollToActiveCell?.() editEnabled.value = false } break case 'ArrowDown': - selectedRange.clear() e.preventDefault() - if (selectedCell.row < unref(data).length - 1) { - selectedCell.row++ + selectedRange.clear() + + if (activeCell.row < unref(data).length - 1) { + activeCell.row++ scrollToActiveCell?.() editEnabled.value = false } break default: { - const rowObj = unref(data)[selectedCell.row] - - const columnObj = unref(fields)[selectedCell.col] + const rowObj = unref(data)[activeCell.row] + const columnObj = unref(fields)[activeCell.col] if ((!unref(editEnabled) || !isTypableInputColumn(columnObj)) && (isMac() ? e.metaKey : e.ctrlKey)) { switch (e.keyCode) { // copy - ctrl/cmd +c case 67: // set clipboard context only if single cell selected - if (rowObj.row[columnObj.title!]) { + if (selectedRange.isSingleCell() && rowObj.row[columnObj.title!]) { clipboardContext = { value: rowObj.row[columnObj.title!], uidt: columnObj.uidt as UITypes, @@ -264,6 +277,7 @@ export function useMultiSelect( } await copyValue() break + // paste - ctrl/cmd + v case 86: try { // handle belongs to column @@ -297,7 +311,7 @@ export function useMultiSelect( (relatedTableMeta as any)!.columns!, ) - return await syncCellData?.({ ...selectedCell, updatedColumnTitle: foreignKeyColumn.title }) + return await syncCellData?.({ ...activeCell, updatedColumnTitle: foreignKeyColumn.title }) } // if it's a virtual column excluding belongs to cell type skip paste @@ -315,9 +329,9 @@ export function useMultiSelect( isMysql.value, ) e.preventDefault() - syncCellData?.(selectedCell) + syncCellData?.(activeCell) } else { - clearCell(selectedCell as { row: number; col: number }, true) + clearCell(activeCell as { row: number; col: number }, true) makeEditable(rowObj, columnObj) } } catch (error: any) { @@ -346,15 +360,18 @@ export function useMultiSelect( } } - useEventListener(document, 'keydown', onKeyDown) + const clearSelectedRange = selectedRange.clear.bind(selectedRange) + + useEventListener(document, 'keydown', handleKeyDown) + useEventListener(document, 'mouseup', handleMouseUp) return { - selectCell, - startSelectRange, - endSelectRange, - clearSelectedRange: selectedRange.clear.bind(selectedRange), + handleMouseDown, + handleMouseOver, + clearSelectedRange, copyValue, isCellSelected, - selectedCell, + activeCell, + handleCellClick, } } diff --git a/packages/nc-gui/lib/types.ts b/packages/nc-gui/lib/types.ts index 9807981404..49ce8be69d 100644 --- a/packages/nc-gui/lib/types.ts +++ b/packages/nc-gui/lib/types.ts @@ -100,3 +100,5 @@ export interface SharedView { export type importFileList = (UploadFile & { data: string | ArrayBuffer })[] export type streamImportFileList = UploadFile[] + +export type Nullable = { [K in keyof T]: T[K] | null } diff --git a/tests/playwright/pages/Dashboard/Grid/index.ts b/tests/playwright/pages/Dashboard/Grid/index.ts index 66a85bee3b..39da3232a3 100644 --- a/tests/playwright/pages/Dashboard/Grid/index.ts +++ b/tests/playwright/pages/Dashboard/Grid/index.ts @@ -1,7 +1,7 @@ import { expect, Locator } from '@playwright/test'; import { DashboardPage } from '..'; import BasePage from '../../Base'; -import { CellPageObject } from '../common/Cell'; +import { CellPageObject, CellProps } from '../common/Cell'; import { ColumnPageObject } from './Column'; import { ToolbarPage } from '../common/Toolbar'; import { ProjectMenuObject } from '../common/ProjectMenu'; @@ -286,4 +286,34 @@ export class GridPage extends BasePage { param.role === 'creator' || param.role === 'editor' ? 1 : 0 ); } + + async selectRange({ start, end }: { start: CellProps; end: CellProps }) { + const startCell = await this.cell.get({ index: start.index, columnHeader: start.columnHeader }); + const endCell = await this.cell.get({ index: end.index, columnHeader: end.columnHeader }); + const page = await this.dashboard.get().page(); + await startCell.hover(); + await page.mouse.down(); + await endCell.hover(); + await page.mouse.up(); + } + + async selectedCount() { + return this.get().locator('.cell.active').count(); + } + + async copyWithKeyboard() { + await this.get().press((await this.isMacOs()) ? 'Meta+C' : 'Control+C'); + await this.verifyToast({ message: 'Copied to clipboard' }); + + return this.getClipboardText(); + } + + async copyWithMouse({ index, columnHeader }: CellProps) { + await this.cell.get({ index, columnHeader }).click({ button: 'right' }); + await this.get().page().getByTestId('context-menu-item-copy').click(); + + await this.verifyToast({ message: 'Copied to clipboard' }); + + return this.getClipboardText(); + } } diff --git a/tests/playwright/pages/Dashboard/common/Cell/index.ts b/tests/playwright/pages/Dashboard/common/Cell/index.ts index 4e1ddb538e..0e1bc8e04b 100644 --- a/tests/playwright/pages/Dashboard/common/Cell/index.ts +++ b/tests/playwright/pages/Dashboard/common/Cell/index.ts @@ -8,6 +8,11 @@ import { CheckboxCellPageObject } from './CheckboxCell'; import { RatingCellPageObject } from './RatingCell'; import { DateCellPageObject } from './DateCell'; +export interface CellProps { + index?: number; + columnHeader: string; +} + export class CellPageObject extends BasePage { readonly parent: GridPage | SharedFormPage; readonly selectOption: SelectOptionCellPageObject; @@ -26,7 +31,7 @@ export class CellPageObject extends BasePage { this.date = new DateCellPageObject(this); } - get({ index, columnHeader }: { index?: number; columnHeader: string }): Locator { + get({ index, columnHeader }: CellProps): Locator { if (this.parent instanceof SharedFormPage) { return this.parent.get().locator(`[data-testid="nc-form-input-cell-${columnHeader}"]`); } else { @@ -34,19 +39,16 @@ export class CellPageObject extends BasePage { } } - async click( - { index, columnHeader }: { index: number; columnHeader: string }, - ...options: Parameters - ) { + async click({ index, columnHeader }: CellProps, ...options: Parameters) { await this.get({ index, columnHeader }).click(...options); await (await this.get({ index, columnHeader }).elementHandle()).waitForElementState('stable'); } - async dblclick({ index, columnHeader }: { index?: number; columnHeader: string }) { + async dblclick({ index, columnHeader }: CellProps) { return await this.get({ index, columnHeader }).dblclick(); } - async fillText({ index, columnHeader, text }: { index?: number; columnHeader: string; text: string }) { + async fillText({ index, columnHeader, text }: CellProps & { text: string }) { await this.dblclick({ index, columnHeader, @@ -67,7 +69,7 @@ export class CellPageObject extends BasePage { } } - async inCellExpand({ index, columnHeader }: { index: number; columnHeader: string }) { + async inCellExpand({ index, columnHeader }: CellProps) { await this.get({ index, columnHeader }).hover(); await this.waitForResponse({ uiAction: this.get({ index, columnHeader }).locator('.nc-action-icon >> nth=0').click(), @@ -76,20 +78,20 @@ export class CellPageObject extends BasePage { }); } - async inCellAdd({ index, columnHeader }: { index: number; columnHeader: string }) { + async inCellAdd({ index, columnHeader }: CellProps) { await this.get({ index, columnHeader }).hover(); await this.get({ index, columnHeader }).locator('.nc-action-icon.nc-plus').click(); } - async verifyCellActiveSelected({ index, columnHeader }: { index: number; columnHeader: string }) { + async verifyCellActiveSelected({ index, columnHeader }: CellProps) { await expect(this.get({ index, columnHeader })).toHaveClass(/active/); } - async verifyCellEditable({ index, columnHeader }: { index: number; columnHeader: string }) { + async verifyCellEditable({ index, columnHeader }: CellProps) { await this.get({ index, columnHeader }).isEditable(); } - async verify({ index, columnHeader, value }: { index: number; columnHeader: string; value: string | string[] }) { + async verify({ index, columnHeader, value }: CellProps & { value: string | string[] }) { const _verify = async text => { await expect .poll(async () => { @@ -115,9 +117,7 @@ export class CellPageObject extends BasePage { index, columnHeader, expectedSrcValue, - }: { - index: number; - columnHeader: string; + }: CellProps & { expectedSrcValue: string; }) { const _verify = async expectedQrCodeImgSrc => { @@ -147,9 +147,7 @@ export class CellPageObject extends BasePage { columnHeader, count, value, - }: { - index: number; - columnHeader: string; + }: CellProps & { count?: number; value: string[]; }) { @@ -166,7 +164,7 @@ export class CellPageObject extends BasePage { } } - async unlinkVirtualCell({ index, columnHeader }: { index: number; columnHeader: string }) { + async unlinkVirtualCell({ index, columnHeader }: CellProps) { const cell = this.get({ index, columnHeader }); await cell.click(); await cell.locator('.nc-icon.unlink-icon').click(); @@ -200,10 +198,7 @@ export class CellPageObject extends BasePage { ); } - async copyToClipboard( - { index, columnHeader }: { index: number; columnHeader: string }, - ...clickOptions: Parameters - ) { + async copyToClipboard({ index, columnHeader }: CellProps, ...clickOptions: Parameters) { await this.get({ index, columnHeader }).click(...clickOptions); await (await this.get({ index, columnHeader }).elementHandle()).waitForElementState('stable'); diff --git a/tests/playwright/tests/cellSelection.spec.ts b/tests/playwright/tests/cellSelection.spec.ts new file mode 100644 index 0000000000..c58f4c3cd0 --- /dev/null +++ b/tests/playwright/tests/cellSelection.spec.ts @@ -0,0 +1,111 @@ +import { expect, test } from '@playwright/test'; +import { DashboardPage } from '../pages/Dashboard'; +import { GridPage } from '../pages/Dashboard/Grid'; +import setup from '../setup'; + +test.describe('Verify cell selection', () => { + let dashboard: DashboardPage, grid: GridPage; + let context: any; + + test.beforeEach(async ({ page }) => { + context = await setup({ page }); + dashboard = new DashboardPage(page, context.project); + grid = dashboard.grid; + }); + + test('#1 when range is selected, it has correct number of selected cells', async () => { + await dashboard.treeView.openTable({ title: 'Country' }); + await grid.selectRange({ + start: { index: 0, columnHeader: 'Country' }, + end: { index: 2, columnHeader: 'City List' }, + }); + + expect(await grid.selectedCount()).toBe(9); + }); + + test('#2 when copied with clipboard, it copies correct text', async () => { + await dashboard.treeView.openTable({ title: 'Country' }); + await grid.selectRange({ + start: { index: 0, columnHeader: 'Country' }, + end: { index: 1, columnHeader: 'LastUpdate' }, + }); + + expect(await grid.copyWithKeyboard()).toBe( + 'Afghanistan \t 2006-02-15 04:44:00\n' + ' Algeria \t 2006-02-15 04:44:00\n' + ); + }); + + test('#3 when copied with mouse, it copies correct text', async () => { + await dashboard.treeView.openTable({ title: 'Country' }); + await grid.selectRange({ + start: { index: 0, columnHeader: 'Country' }, + end: { index: 1, columnHeader: 'LastUpdate' }, + }); + + expect(await grid.copyWithMouse({ index: 0, columnHeader: 'Country' })).toBe( + 'Afghanistan \t 2006-02-15 04:44:00\n' + ' Algeria \t 2006-02-15 04:44:00\n' + ); + }); + + // FIXME: this is edge case, better be moved to integration tests + test('#4 when cell inside selection range is clicked, it clears previous selection', async () => { + await dashboard.treeView.openTable({ title: 'Country' }); + await grid.selectRange({ + start: { index: 0, columnHeader: 'Country' }, + end: { index: 2, columnHeader: 'City List' }, + }); + + expect(await grid.selectedCount()).toBe(9); + + await grid.cell.get({ index: 0, columnHeader: 'Country' }).click(); + + expect(await grid.selectedCount()).toBe(1); + expect(await grid.cell.verifyCellActiveSelected({ index: 0, columnHeader: 'Country' })); + }); + + // FIXME: this is edge case, better be moved to integration tests + test('#5 when cell outside selection range is clicked, it clears previous selection', async () => { + await dashboard.treeView.openTable({ title: 'Country' }); + await grid.selectRange({ + start: { index: 0, columnHeader: 'Country' }, + end: { index: 2, columnHeader: 'City List' }, + }); + + expect(await grid.selectedCount()).toBe(9); + + await grid.cell.get({ index: 5, columnHeader: 'Country' }).click(); + + expect(await grid.selectedCount()).toBe(1); + expect(await grid.cell.verifyCellActiveSelected({ index: 5, columnHeader: 'Country' })); + }); + + // FIXME: this is edge case, better be moved to integration tests + test('#6 when selection ends on locked field, it still works as expected', async () => { + await dashboard.treeView.openTable({ title: 'Country' }); + await dashboard.grid.toolbar.fields.toggleShowSystemFields(); + await grid.selectRange({ + start: { index: 2, columnHeader: 'City List' }, + end: { index: 0, columnHeader: 'CountryId' }, + }); + + expect(await grid.selectedCount()).toBe(12); + + await grid.cell.get({ index: 1, columnHeader: 'Country' }).click(); + + expect(await grid.selectedCount()).toBe(1); + expect(await grid.cell.verifyCellActiveSelected({ index: 1, columnHeader: 'Country' })); + }); + + // FIXME: this is edge case, better be moved to integration tests + test('#7 when navigated with keyboard, only active cell is affected', async ({ page }) => { + await dashboard.treeView.openTable({ title: 'Country' }); + await grid.selectRange({ + start: { index: 0, columnHeader: 'Country' }, + end: { index: 2, columnHeader: 'City List' }, + }); + + await page.keyboard.press('ArrowRight'); + expect(await grid.selectedCount()).toBe(1); + expect(await grid.cell.verifyCellActiveSelected({ index: 0, columnHeader: 'LastUpdate' })); + }); +}); From f869c8fdc6a32bc54e654d34a3de93edbb197cc0 Mon Sep 17 00:00:00 2001 From: Ekaterina Balakina Date: Tue, 20 Dec 2022 17:41:04 +0100 Subject: [PATCH 2/3] fix tests --- tests/playwright/tests/cellSelection.spec.ts | 26 +++++++++----------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/tests/playwright/tests/cellSelection.spec.ts b/tests/playwright/tests/cellSelection.spec.ts index c58f4c3cd0..9ef20b7fb9 100644 --- a/tests/playwright/tests/cellSelection.spec.ts +++ b/tests/playwright/tests/cellSelection.spec.ts @@ -14,36 +14,34 @@ test.describe('Verify cell selection', () => { }); test('#1 when range is selected, it has correct number of selected cells', async () => { - await dashboard.treeView.openTable({ title: 'Country' }); + await dashboard.treeView.openTable({ title: 'Customer' }); await grid.selectRange({ - start: { index: 0, columnHeader: 'Country' }, - end: { index: 2, columnHeader: 'City List' }, + start: { index: 0, columnHeader: 'FirstName' }, + end: { index: 2, columnHeader: 'Email' }, }); expect(await grid.selectedCount()).toBe(9); }); test('#2 when copied with clipboard, it copies correct text', async () => { - await dashboard.treeView.openTable({ title: 'Country' }); + await dashboard.treeView.openTable({ title: 'Customer' }); await grid.selectRange({ - start: { index: 0, columnHeader: 'Country' }, - end: { index: 1, columnHeader: 'LastUpdate' }, + start: { index: 0, columnHeader: 'FirstName' }, + end: { index: 1, columnHeader: 'LastName' }, }); - expect(await grid.copyWithKeyboard()).toBe( - 'Afghanistan \t 2006-02-15 04:44:00\n' + ' Algeria \t 2006-02-15 04:44:00\n' - ); + expect(await grid.copyWithKeyboard()).toBe('MARY \t SMITH\n' + ' PATRICIA \t JOHNSON\n'); }); test('#3 when copied with mouse, it copies correct text', async () => { - await dashboard.treeView.openTable({ title: 'Country' }); + await dashboard.treeView.openTable({ title: 'Customer' }); await grid.selectRange({ - start: { index: 0, columnHeader: 'Country' }, - end: { index: 1, columnHeader: 'LastUpdate' }, + start: { index: 0, columnHeader: 'FirstName' }, + end: { index: 1, columnHeader: 'LastName' }, }); - expect(await grid.copyWithMouse({ index: 0, columnHeader: 'Country' })).toBe( - 'Afghanistan \t 2006-02-15 04:44:00\n' + ' Algeria \t 2006-02-15 04:44:00\n' + expect(await grid.copyWithMouse({ index: 0, columnHeader: 'FirstName' })).toBe( + 'MARY \t SMITH\n' + ' PATRICIA \t JOHNSON\n' ); }); From fc9a4caae33594ac0a066fb738723285e75b4a72 Mon Sep 17 00:00:00 2001 From: Ekaterina Balakina Date: Wed, 21 Dec 2022 09:38:45 +0100 Subject: [PATCH 3/3] review fixes --- packages/nc-gui/components/smartsheet/Grid.vue | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/nc-gui/components/smartsheet/Grid.vue b/packages/nc-gui/components/smartsheet/Grid.vue index 4dc08c0990..304daa4b8e 100644 --- a/packages/nc-gui/components/smartsheet/Grid.vue +++ b/packages/nc-gui/components/smartsheet/Grid.vue @@ -229,7 +229,6 @@ const { isCellSelected, activeCell, handleMouseDown, handleMouseOver, handleCell case 'ArrowUp': e.preventDefault() clearSelectedRange() - $e('c:shortcut', { key: 'CTRL + ArrowUp' }) activeCell.row = 0 activeCell.col = activeCell.col ?? 0 scrollToCell?.()