From c95fd09b67c406edfb0380cb573914d0a1cb1d3c Mon Sep 17 00:00:00 2001 From: YuHong Date: Tue, 21 May 2024 14:57:06 +0800 Subject: [PATCH 1/3] fix: move row&col effects on filter-range --- .../sheets-filter-ui.controller.ts | 19 ++++- packages/sheets-filter-ui/src/locale/en-US.ts | 3 + packages/sheets-filter-ui/src/locale/ru-RU.ts | 3 + packages/sheets-filter-ui/src/locale/zh-CN.ts | 3 + .../controllers/sheets-filter.controller.ts | 80 +++++++++++++++---- .../src/services/sheet-filter.service.ts | 7 ++ 6 files changed, 96 insertions(+), 19 deletions(-) diff --git a/packages/sheets-filter-ui/src/controllers/sheets-filter-ui.controller.ts b/packages/sheets-filter-ui/src/controllers/sheets-filter-ui.controller.ts index 4021ab69e4b..a3350ef2a24 100644 --- a/packages/sheets-filter-ui/src/controllers/sheets-filter-ui.controller.ts +++ b/packages/sheets-filter-ui/src/controllers/sheets-filter-ui.controller.ts @@ -15,9 +15,9 @@ */ import type { Nullable } from '@univerjs/core'; -import { ICommandService, IContextService, LifecycleStages, OnLifecycle, RxDisposable, UniverInstanceType } from '@univerjs/core'; +import { ICommandService, IContextService, LifecycleStages, LocaleService, OnLifecycle, RxDisposable, UniverInstanceType } from '@univerjs/core'; import type { IMenuItemFactory } from '@univerjs/ui'; -import { ComponentManager, IMenuService, IShortcutService } from '@univerjs/ui'; +import { ComponentManager, IMenuService, IMessageService, IShortcutService } from '@univerjs/ui'; import type { IDisposable } from '@wendellhu/redi'; import { Inject, Injector } from '@wendellhu/redi'; @@ -26,6 +26,8 @@ import { SheetCanvasPopManagerService } from '@univerjs/sheets-ui'; import { FilterSingle } from '@univerjs/icons'; import { IRenderManagerService } from '@univerjs/engine-render'; +import { SheetsFilterService } from '@univerjs/sheets-filter'; +import { MessageType } from '@univerjs/design'; import { ClearSheetsFilterCriteriaCommand, ReCalcSheetsFilterCommand, SetSheetsFilterCriteriaCommand, SmartToggleSheetsFilterCommand } from '../commands/sheets-filter.command'; import { FilterPanel } from '../views/components/SheetsFilterPanel'; import { ChangeFilterByOperation, CloseFilterPanelOperation, FILTER_PANEL_OPENED_KEY, OpenFilterPanelOperation } from '../commands/sheets-filter.operation'; @@ -46,11 +48,14 @@ export class SheetsFilterUIController extends RxDisposable { @Inject(ComponentManager) private readonly _componentManager: ComponentManager, @Inject(SheetsFilterPanelService) private readonly _sheetsFilterPanelService: SheetsFilterPanelService, @Inject(SheetCanvasPopManagerService) private _sheetCanvasPopupService: SheetCanvasPopManagerService, + @Inject(SheetsFilterService) private _sheetsFilterService: SheetsFilterService, + @Inject(LocaleService) private _localeService: LocaleService, @IRenderManagerService private readonly _renderManagerService: IRenderManagerService, @IShortcutService private readonly _shortcutService: IShortcutService, @ICommandService private readonly _commandService: ICommandService, @IMenuService private readonly _menuService: IMenuService, - @IContextService private readonly _contextService: IContextService + @IContextService private readonly _contextService: IContextService, + @IMessageService private readonly _messageService: IMessageService ) { super(); @@ -111,6 +116,14 @@ export class SheetsFilterUIController extends RxDisposable { this._closeFilterPopup(); } })); + this.disposeWithMe(this._sheetsFilterService.errorMsg$.subscribe((content) => { + if (content) { + this._messageService.show({ + type: MessageType.Error, + content: this._localeService.t(content), + }); + } + })); } private _initRenderControllers(): void { diff --git a/packages/sheets-filter-ui/src/locale/en-US.ts b/packages/sheets-filter-ui/src/locale/en-US.ts index b6d42a4a0b5..ba7c6bfa709 100644 --- a/packages/sheets-filter-ui/src/locale/en-US.ts +++ b/packages/sheets-filter-ui/src/locale/en-US.ts @@ -64,6 +64,9 @@ const locale: typeof zhCN = { 'not-between': 'Not Between', custom: 'Custom', }, + msg: { + 'filter-header-forbidden': 'You can\'t move the header row of a filter.', + }, }, }; diff --git a/packages/sheets-filter-ui/src/locale/ru-RU.ts b/packages/sheets-filter-ui/src/locale/ru-RU.ts index bb4b6a20921..a1a66f0b975 100644 --- a/packages/sheets-filter-ui/src/locale/ru-RU.ts +++ b/packages/sheets-filter-ui/src/locale/ru-RU.ts @@ -64,6 +64,9 @@ const locale: typeof zhCN = { 'not-between': 'Не между', custom: 'Пользовательский', }, + msg: { + 'filter-header-forbidden': 'Вы не можете переместить строку заголовка фильтра.', + }, }, }; diff --git a/packages/sheets-filter-ui/src/locale/zh-CN.ts b/packages/sheets-filter-ui/src/locale/zh-CN.ts index f37ab9534a0..6b2d50e0529 100644 --- a/packages/sheets-filter-ui/src/locale/zh-CN.ts +++ b/packages/sheets-filter-ui/src/locale/zh-CN.ts @@ -62,6 +62,9 @@ const locale = { 'not-between': '不介于', custom: '自定义', }, + msg: { + 'filter-header-forbidden': '无法移动筛选行头', + }, }, }; diff --git a/packages/sheets-filter/src/controllers/sheets-filter.controller.ts b/packages/sheets-filter/src/controllers/sheets-filter.controller.ts index e7ab380d1c0..ed98b858272 100644 --- a/packages/sheets-filter/src/controllers/sheets-filter.controller.ts +++ b/packages/sheets-filter/src/controllers/sheets-filter.controller.ts @@ -17,7 +17,7 @@ import type { ICommandInfo, IMutationInfo, IObjectArrayPrimitiveType, Nullable } from '@univerjs/core'; import { Disposable, DisposableCollection, ICommandService, IUniverInstanceService, LifecycleStages, moveMatrixArray, OnLifecycle, Rectangle } from '@univerjs/core'; import type { EffectRefRangeParams, IAddWorksheetMergeMutationParams, IInsertColCommandParams, IInsertRowCommandParams, IInsertRowMutationParams, IMoveColsCommandParams, IMoveRangeCommandParams, IMoveRowsCommandParams, IRemoveColMutationParams, IRemoveRowsMutationParams, IRemoveSheetCommandParams, ISetWorksheetActivateCommandParams, ISheetCommandSharedParams } from '@univerjs/sheets'; -import { EffectRefRangId, InsertColCommand, InsertRowCommand, InsertRowMutation, INTERCEPTOR_POINT, MoveRangeCommand, RefRangeService, RemoveColCommand, RemoveRowCommand, RemoveRowMutation, RemoveSheetCommand, SetWorksheetActivateCommand, SheetInterceptorService } from '@univerjs/sheets'; +import { EffectRefRangId, getSheetCommandTarget, InsertColCommand, InsertRowCommand, InsertRowMutation, INTERCEPTOR_POINT, MoveRangeCommand, MoveRowsCommand, RefRangeService, RemoveColCommand, RemoveRowCommand, RemoveRowMutation, RemoveSheetCommand, SetWorksheetActivateCommand, SheetInterceptorService } from '@univerjs/sheets'; import { Inject } from '@wendellhu/redi'; import { SheetsFilterService } from '../services/sheet-filter.service'; @@ -41,6 +41,7 @@ export class SheetsFilterController extends Disposable { this._initRowFilteredInterceptor(); this._initInterceptors(); this._commandExecutedListener(); + this._initErrorHandling(); } private _initCommands(): void { @@ -389,6 +390,7 @@ export class SheetsFilterController extends Disposable { }; } + // eslint-disable-next-line max-lines-per-function private _handleMoveColsCommand(config: IMoveColsCommandParams, unitId: string, subUnitId: string) { const filterModel = this._sheetsFilterService.getFilterModel(unitId, subUnitId); const filterRange = filterModel?.getRange() ?? null; @@ -413,24 +415,43 @@ export class SheetsFilterController extends Disposable { } moveMatrixArray(fromRange.startColumn, fromRange.endColumn - fromRange.startColumn + 1, toRange.startColumn, filterCol); + let startBorder = filterRange.startColumn; + let endBorder = filterRange.endColumn; + + // border will change if first col or last col moves. + if (startColumn >= fromRange.startColumn && startColumn <= fromRange.endColumn + && endColumn > fromRange.endColumn + && toRange.startColumn > endColumn + ) { + startBorder = fromRange.endColumn + 1; + } + if (endColumn >= fromRange.startColumn && endColumn <= fromRange.endColumn + && startColumn < fromRange.startColumn + && toRange.startColumn <= startColumn) { + endBorder = fromRange.startColumn - 1; + } + const numberCols = Object.keys(filterCol).map((col) => Number(col)) as number[]; - const newEnd = Math.max(...numberCols); - const newStart = Math.min(...numberCols); + // find the start col & end col of new filter range by border. + const newEnd = numberCols.find((col) => filterCol[col].colIndex === endBorder) as number; + const newStart = numberCols.find((col) => filterCol[col].colIndex === startBorder) as number; numberCols.forEach((col) => { const { colIndex: oldColIndex, filter } = filterCol[col]; const newColIndex = col; - if (filter) { - const setCriteriaMutationParams: ISetSheetsFilterCriteriaMutationParams = { - unitId, - subUnitId, - col: newColIndex, - criteria: { ...filter.serialize(), colId: newColIndex }, - }; - redos.push({ id: SetSheetsFilterCriteriaMutation.id, params: setCriteriaMutationParams }); - undos.push({ id: RemoveSheetsFilterMutation.id, params: { unitId, subUnitId, col: newColIndex, criteria: { ...filterModel.getFilterColumn(newColIndex)?.serialize(), colId: newColIndex } } }); + if (filter) { + if (newColIndex >= newStart && newColIndex <= newEnd) { + const setCriteriaMutationParams: ISetSheetsFilterCriteriaMutationParams = { + unitId, + subUnitId, + col: newColIndex, + criteria: { ...filter.serialize(), colId: newColIndex }, + }; + redos.push({ id: SetSheetsFilterCriteriaMutation.id, params: setCriteriaMutationParams }); + undos.push({ id: RemoveSheetsFilterMutation.id, params: { unitId, subUnitId, col: newColIndex, criteria: { ...filterModel.getFilterColumn(newColIndex)?.serialize(), colId: newColIndex } } }); + } if (!filterCol[oldColIndex]?.filter) { const setCriteriaMutationParams: ISetSheetsFilterCriteriaMutationParams = { unitId, @@ -479,18 +500,28 @@ export class SheetsFilterController extends Disposable { } const redos: IMutationInfo[] = []; const undos: IMutationInfo[] = []; - const filterRow: IObjectArrayPrimitiveType<{ offset: number }> = {}; + const filterRow: IObjectArrayPrimitiveType<{ oldIndex: number }> = {}; for (let row = startRow; row <= endRow; row++) { filterRow[row] = { - offset: row - startRow, + oldIndex: row, }; } + const startBorder = startRow; + let endBorder = endRow; + + // only need to deal with endBorder, startRow will not be moved. + if (endRow >= fromRange.startRow && endRow <= fromRange.endRow + && startRow < fromRange.startRow + && toRange.startRow <= startRow) { + endBorder = fromRange.startRow - 1; + } moveMatrixArray(fromRange.startRow, fromRange.endRow - fromRange.startRow + 1, toRange.startRow, filterRow); const numberRows = Object.keys(filterRow).map((row) => Number(row)); - const newEnd = Math.max(...numberRows); - const newStart = Math.min(...numberRows); + const newEnd = numberRows.find((row) => filterRow[row].oldIndex === endBorder) as number; + const newStart = numberRows.find((row) => filterRow[row].oldIndex === startBorder) as number; + if (startRow !== newStart || endRow !== newEnd) { const setFilterRangeMutationParams: ISetSheetsFilterRangeMutationParams = { unitId, @@ -756,5 +787,22 @@ export class SheetsFilterController extends Disposable { // } })); } + + private _initErrorHandling() { + this.disposeWithMe(this._commandService.beforeCommandExecuted((command) => { + const params = command.params as IMoveRowsCommandParams; + const target = getSheetCommandTarget(this._univerInstanceService); + if (!target) return; + + const { subUnitId, unitId } = target; + const filterModel = this._sheetsFilterService.getFilterModel(unitId, subUnitId); + if (!filterModel) return; + const filterRange = filterModel.getRange(); + if (command.id === MoveRowsCommand.id && params.fromRange.startRow <= filterRange.startRow && params.fromRange.endRow >= filterRange.startRow) { + this._sheetsFilterService.setFilterErrorMsg('sheets-filter.msg.filter-header-forbidden'); + throw new Error('[SheetsFilterController]: Cannot move header row of filter'); + } + })); + } } diff --git a/packages/sheets-filter/src/services/sheet-filter.service.ts b/packages/sheets-filter/src/services/sheet-filter.service.ts index e88e9ca4c38..b794fb465ea 100644 --- a/packages/sheets-filter/src/services/sheet-filter.service.ts +++ b/packages/sheets-filter/src/services/sheet-filter.service.ts @@ -59,6 +59,9 @@ export class SheetsFilterService extends Disposable { private readonly _loadedUnitId$ = new BehaviorSubject>(null); readonly loadedUnitId$ = this._loadedUnitId$.asObservable(); + private readonly _errorMsg$ = new BehaviorSubject>(null); + readonly errorMsg$ = this._errorMsg$.asObservable(); + private readonly _activeFilterModel$ = new BehaviorSubject>(null); /** An observable value emitting the current Workbook's active Worksheet's filter model (if there is one). */ readonly activeFilterModel$ = this._activeFilterModel$.asObservable(); @@ -116,6 +119,10 @@ export class SheetsFilterService extends Disposable { return false; } + setFilterErrorMsg(content: string) { + this._errorMsg$.next(content); + } + private _updateActiveFilterModel() { let workbook: Nullable; try { From 453e2b487acb0139e163a93a920aaa29ee19e6d0 Mon Sep 17 00:00:00 2001 From: YuHong Date: Tue, 21 May 2024 15:50:31 +0800 Subject: [PATCH 2/3] fix: test --- .../src/models/__tests__/filter-interceptor.spec.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/sheets-filter/src/models/__tests__/filter-interceptor.spec.ts b/packages/sheets-filter/src/models/__tests__/filter-interceptor.spec.ts index 327ec35d8c9..4a5a97fcc6e 100644 --- a/packages/sheets-filter/src/models/__tests__/filter-interceptor.spec.ts +++ b/packages/sheets-filter/src/models/__tests__/filter-interceptor.spec.ts @@ -117,7 +117,7 @@ describe('Test "Filter Interceptor"', () => { selectionManagerService.setCurrentSelection({ unitId: 'workbookId', sheetId: 'worksheetId', pluginName: NORMAL_SELECTION_PLUGIN_NAME }); await commandService.executeCommand(SetSelectionsOperation.id, { unitId: 'workbookId', subUnitId: 'worksheetId', pluginName: NORMAL_SELECTION_PLUGIN_NAME, selections: [{ style: null, range: { startColumn: 1, endColumn: 1, startRow: 0, endRow: 4, rangeType: RANGE_TYPE.COLUMN }, primary: {} }] } as ISetSelectionsOperationParams); await commandService.executeCommand(MoveColsCommand.id, moveColCommandParams); - expect(sheetsFilterService.getFilterModel('workbookId', 'worksheetId')!.getRange()).toStrictEqual({ startColumn: 1, endColumn: 2, startRow: 1, endRow: 2 }); + expect(sheetsFilterService.getFilterModel('workbookId', 'worksheetId')!.getRange()).toStrictEqual({ startColumn: 1, endColumn: 1, startRow: 1, endRow: 2 }); expect(sheetsFilterService.getFilterModel('workbookId', 'worksheetId')!.getAllFilterColumns().map((x) => x[0])).toStrictEqual([1]); }); it('move col command, filter column move to right', async () => { @@ -132,13 +132,13 @@ describe('Test "Filter Interceptor"', () => { }); it('move row command', async () => { - const moveColCommandParams = { unitId: 'workbookId', subUnitId: 'worksheetId', fromRange: { startColumn: 0, endColumn: 3, startRow: 1, endRow: 1, type: RANGE_TYPE.ROW, - }, toRange: { startColumn: 0, endColumn: 3, startRow: 3, endRow: 3, type: RANGE_TYPE.ROW }, direction: Direction.RIGHT } as IMoveRowsCommandParams; + const moveRowCommandParams = { unitId: 'workbookId', subUnitId: 'worksheetId', fromRange: { startColumn: 0, endColumn: 3, startRow: 2, endRow: 2, type: RANGE_TYPE.ROW, + }, toRange: { startColumn: 0, endColumn: 3, startRow: 4, endRow: 4, type: RANGE_TYPE.ROW }, direction: Direction.RIGHT } as IMoveRowsCommandParams; const selectionManagerService = get(SelectionManagerService); selectionManagerService.setCurrentSelection({ unitId: 'workbookId', sheetId: 'worksheetId', pluginName: NORMAL_SELECTION_PLUGIN_NAME }); - await commandService.executeCommand(SetSelectionsOperation.id, { unitId: 'workbookId', subUnitId: 'worksheetId', pluginName: NORMAL_SELECTION_PLUGIN_NAME, selections: [{ style: null, range: { startColumn: 0, endColumn: 3, startRow: 1, endRow: 1, rangeType: RANGE_TYPE.ROW }, primary: {} }] } as ISetSelectionsOperationParams); - await commandService.executeCommand(MoveRowsCommand.id, moveColCommandParams); - expect(sheetsFilterService.getFilterModel('workbookId', 'worksheetId')!.getRange()).toStrictEqual({ startColumn: 1, endColumn: 2, startRow: 1, endRow: 2 }); + await commandService.executeCommand(SetSelectionsOperation.id, { unitId: 'workbookId', subUnitId: 'worksheetId', pluginName: NORMAL_SELECTION_PLUGIN_NAME, selections: [{ style: null, range: { startColumn: 0, endColumn: 3, startRow: 2, endRow: 2, rangeType: RANGE_TYPE.ROW }, primary: {} }] } as ISetSelectionsOperationParams); + await commandService.executeCommand(MoveRowsCommand.id, moveRowCommandParams); + expect(sheetsFilterService.getFilterModel('workbookId', 'worksheetId')!.getRange()).toStrictEqual({ startColumn: 1, endColumn: 2, startRow: 1, endRow: 3 }); expect(sheetsFilterService.getFilterModel('workbookId', 'worksheetId')!.getAllFilterColumns().map((x) => x[0])).toStrictEqual([2]); }); }); From 4646fcc3b3efc0df204daf3e65c8dfe11aa7c50d Mon Sep 17 00:00:00 2001 From: YuHong Date: Wed, 22 May 2024 16:37:18 +0800 Subject: [PATCH 3/3] fix: border moves --- .../src/controllers/sheets-filter.controller.ts | 14 ++++++++------ packages/sheets-sort/src/services/utils.ts | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 6 deletions(-) create mode 100644 packages/sheets-sort/src/services/utils.ts diff --git a/packages/sheets-filter/src/controllers/sheets-filter.controller.ts b/packages/sheets-filter/src/controllers/sheets-filter.controller.ts index ed98b858272..0027d854f8b 100644 --- a/packages/sheets-filter/src/controllers/sheets-filter.controller.ts +++ b/packages/sheets-filter/src/controllers/sheets-filter.controller.ts @@ -420,14 +420,15 @@ export class SheetsFilterController extends Disposable { // border will change if first col or last col moves. if (startColumn >= fromRange.startColumn && startColumn <= fromRange.endColumn - && endColumn > fromRange.endColumn - && toRange.startColumn > endColumn + && toRange.startColumn > fromRange.startColumn + && fromRange.endColumn < endColumn ) { startBorder = fromRange.endColumn + 1; } if (endColumn >= fromRange.startColumn && endColumn <= fromRange.endColumn - && startColumn < fromRange.startColumn - && toRange.startColumn <= startColumn) { + && toRange.startColumn < fromRange.startColumn + && fromRange.startColumn > startColumn + ) { endBorder = fromRange.startColumn - 1; } @@ -511,8 +512,9 @@ export class SheetsFilterController extends Disposable { // only need to deal with endBorder, startRow will not be moved. if (endRow >= fromRange.startRow && endRow <= fromRange.endRow - && startRow < fromRange.startRow - && toRange.startRow <= startRow) { + && toRange.startRow < fromRange.startRow + && fromRange.startRow > startRow + ) { endBorder = fromRange.startRow - 1; } diff --git a/packages/sheets-sort/src/services/utils.ts b/packages/sheets-sort/src/services/utils.ts new file mode 100644 index 00000000000..cf732b1e137 --- /dev/null +++ b/packages/sheets-sort/src/services/utils.ts @@ -0,0 +1,16 @@ +/** + * Copyright 2023-present DreamNum Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +