Skip to content

Commit

Permalink
fix: hyper-link ref-range error on filter (#3135)
Browse files Browse the repository at this point in the history
  • Loading branch information
weird94 authored Aug 22, 2024
1 parent 9a62d77 commit 014dd67
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 10 deletions.
2 changes: 1 addition & 1 deletion packages/design/src/components/popup/index.module.less
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
.popup-absolute {
position: absolute;
z-index: 100;
z-index: 1000;
top: -9999px;
left: -9999px;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import type { IDisposable, IRange, Nullable } from '@univerjs/core';
import { Disposable, ICommandService, Inject, isValidRange, LifecycleStages, OnLifecycle, sequenceExecuteAsync, toDisposable } from '@univerjs/core';
import type { EffectRefRangeParams } from '@univerjs/sheets';
import { handleDefaultRangeChangeWithEffectRefCommandsSkipNoInterests, RefRangeService, SheetsSelectionsService } from '@univerjs/sheets';
import { handleCommonRangeChangeWithEffectRefCommandsSkipNoInterests, handleDefaultRangeChangeWithEffectRefCommandsSkipNoInterests, RefRangeService, SheetsSelectionsService } from '@univerjs/sheets';
import type { IAddHyperLinkMutationParams, ICellHyperLink, IRemoveHyperLinkMutationParams, IUpdateHyperLinkMutationParams, IUpdateHyperLinkRefMutationParams } from '@univerjs/sheets-hyper-link';
import { AddHyperLinkMutation, HyperLinkModel, RemoveHyperLinkMutation, UpdateHyperLinkMutation, UpdateHyperLinkRefMutation } from '@univerjs/sheets-hyper-link';
import { deserializeRangeWithSheet, serializeRange } from '@univerjs/engine-formula';
Expand Down Expand Up @@ -105,7 +105,8 @@ export class SheetsHyperLinkRefRangeController extends Disposable {
};

const handleRefRangeChange = (commandInfo: EffectRefRangeParams) => {
const resultRange = handleDefaultRangeChangeWithEffectRefCommandsSkipNoInterests(oldRange, commandInfo, { selectionManagerService: this._selectionManagerService });
const resultRanges = handleCommonRangeChangeWithEffectRefCommandsSkipNoInterests(oldRange, commandInfo, { selectionManagerService: this._selectionManagerService });
const resultRange = Array.isArray(resultRanges) ? resultRanges[0] : resultRanges;
if (resultRange && resultRange.startColumn === oldRange.startColumn && resultRange.startRow === oldRange.startRow) {
return {
undos: [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import type { IDisposable, IRange, Nullable } from '@univerjs/core';
import { Disposable, ICommandService, Inject, LifecycleStages, OnLifecycle, sequenceExecuteAsync, toDisposable } from '@univerjs/core';
import type { EffectRefRangeParams } from '@univerjs/sheets';
import { handleDefaultRangeChangeWithEffectRefCommandsSkipNoInterests, RefRangeService, SheetsSelectionsService } from '@univerjs/sheets';
import { handleCommonRangeChangeWithEffectRefCommandsSkipNoInterests, RefRangeService, SheetsSelectionsService } from '@univerjs/sheets';
import type { IAddCommentMutationParams, IUpdateCommentRefMutationParams } from '@univerjs/thread-comment';
import { AddCommentMutation, DeleteCommentMutation, ThreadCommentModel, UpdateCommentRefMutation } from '@univerjs/thread-comment';
import { serializeRange, singleReferenceToGrid } from '@univerjs/engine-formula';
Expand Down Expand Up @@ -115,7 +115,8 @@ export class SheetsThreadCommentRefRangeController extends Disposable {
this._disposableMap.set(
this._getIdWithUnitId(unitId, subUnitId, commentId),
this._refRangeService.registerRefRange(oldRange, (commandInfo: EffectRefRangeParams) => {
const resultRange = handleDefaultRangeChangeWithEffectRefCommandsSkipNoInterests(oldRange, commandInfo, { selectionManagerService: this._selectionManagerService });
const resultRanges = handleCommonRangeChangeWithEffectRefCommandsSkipNoInterests(oldRange, commandInfo, { selectionManagerService: this._selectionManagerService });
const resultRange = Array.isArray(resultRanges) ? resultRanges[0] : resultRanges;
if (resultRange && resultRange.startColumn === oldRange.startColumn && resultRange.startRow === oldRange.startRow) {
return {
undos: [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ import { getSheetCommandTarget } from './utils/target-util';
export interface IRemoveRowColCommandParams {
range: IRange;
}

export interface IRemoveRowColCommandInterceptParams extends IRemoveRowColCommandParams {
ranges?: IRange[];
}

export const RemoveRowCommandId = 'sheet.command.remove-row';
/**
* This command would remove the selected rows. These selected rows can be non-continuous.
Expand Down Expand Up @@ -102,7 +107,7 @@ export const RemoveRowCommand: ICommand<IRemoveRowColCommandParams> = {

const canPerform = await sheetInterceptorService.beforeCommandExecute({
id: RemoveRowCommand.id,
params: { range: totalRange } as IRemoveRowColCommandParams,
params: { range: totalRange, ranges } as IRemoveRowColCommandInterceptParams,
});

if (!canPerform) {
Expand Down Expand Up @@ -136,7 +141,7 @@ export const RemoveRowCommand: ICommand<IRemoveRowColCommandParams> = {

const intercepted = sheetInterceptorService.onCommandExecute({
id: RemoveRowCommand.id,
params: { range: totalRange } as IRemoveRowColCommandParams,
params: { range: totalRange, ranges } as IRemoveRowColCommandInterceptParams,
});

const commandService = accessor.get(ICommandService);
Expand Down
3 changes: 2 additions & 1 deletion packages/sheets/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ export {
handleCommonDefaultRangeChangeWithEffectRefCommands,
handleDefaultRangeChangeWithEffectRefCommands,
handleDefaultRangeChangeWithEffectRefCommandsSkipNoInterests,
handleCommonRangeChangeWithEffectRefCommandsSkipNoInterests,
} from './services/ref-range/util';
export { INTERCEPTOR_POINT, InterceptCellContentPriority } from './services/sheet-interceptor/interceptor-const';
export { SheetInterceptorService } from './services/sheet-interceptor/sheet-interceptor.service';
Expand Down Expand Up @@ -206,7 +207,7 @@ export {
type IMoveColsCommandParams,
} from './commands/commands/move-rows-cols.command';
export { RemoveDefinedNameCommand } from './commands/commands/remove-defined-name.command';
export { RemoveRowCommand, RemoveColCommand, type IRemoveRowColCommandParams } from './commands/commands/remove-row-col.command';
export { RemoveRowCommand, RemoveColCommand, type IRemoveRowColCommandParams, type IRemoveRowColCommandInterceptParams } from './commands/commands/remove-row-col.command';
export { RemoveSheetCommand, type IRemoveSheetCommandParams } from './commands/commands/remove-sheet.command';
export { RemoveWorksheetMergeCommand } from './commands/commands/remove-worksheet-merge.command';
export { ReorderRangeCommand, type IReorderRangeCommandParams } from './commands/commands/reorder-range.command';
Expand Down
16 changes: 16 additions & 0 deletions packages/sheets/src/services/ref-range/__tests__/utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
handleMoveRangeCommon,
handleMoveRows,
handleMoveRowsCommon,
handleRemoveRowCommon,
runRefRangeMutations,
} from '../util';
import { RemoveSheetMutation } from '../../../commands/mutations/remove-sheet.mutation';
Expand Down Expand Up @@ -1452,5 +1453,20 @@ describe('test different situations of adjustRangeOnMutation', () => {
const result = adjustRangeOnMutation(range, mutation);
expect(result).toEqual({ startRow: 7, endRow: 9, startColumn: 1, endColumn: 1 });
});

it('should be delete', () => {
const targetRange: IRange = { startRow: 5, endRow: 7, startColumn: 1, endColumn: 1 };
const resultRange = handleRemoveRowCommon({ range: { ...targetRange } }, targetRange);
expect(resultRange.length).toBe(0);

const resultRange2 = handleRemoveRowCommon({ range: { ...targetRange } }, { ...targetRange, startRow: 4 });
expect(resultRange2).toEqual([{ startRow: 4, endRow: 4, startColumn: 1, endColumn: 1 }]);
});

it('should not be delete ', () => {
const targetRange: IRange = { startRow: 5, endRow: 7, startColumn: 1, endColumn: 1 };
const resultRange = handleRemoveRowCommon({ range: { ...targetRange }, ranges: [{ startRow: 5, endRow: 5, startColumn: 1, endColumn: 1 }, { startRow: 6, endRow: 6, startColumn: 1, endColumn: 1 }] }, targetRange);
expect(resultRange).toEqual([{ startRow: 5, endRow: 5, startColumn: 1, endColumn: 1 }]);
});
});
});
36 changes: 34 additions & 2 deletions packages/sheets/src/services/ref-range/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import type { ISheetCommandSharedParams } from '../../commands/utils/interface';
import type { SheetsSelectionsService } from '../selections/selection-manager.service';
import { DeleteRangeMoveLeftCommand } from '../../commands/commands/delete-range-move-left.command';
import { DeleteRangeMoveUpCommand } from '../../commands/commands/delete-range-move-up.command';
import type { IRemoveRowColCommandInterceptParams } from '../../commands/commands/remove-row-col.command';
import type {
EffectRefRangeParams,
IDeleteRangeMoveLeftCommand,
Expand Down Expand Up @@ -872,6 +873,24 @@ export const handleDeleteRangeMoveUpCommon = (param: IDeleteRangeMoveUpCommand,
return queryObjectMatrix(matrix, (v) => v === 1);
};

export const handleRemoveRowCommon = (param: IRemoveRowColCommandInterceptParams, targetRange: IRange) => {
const ranges = param.ranges ?? [param.range];
const matrix = new ObjectMatrix();

Range.foreach(targetRange, (row, col) => {
matrix.setValue(row, col, 1);
});

ranges.forEach((range) => {
const startRow = range.startRow;
const endRow = range.endRow;
const count = endRow - startRow + 1;
matrix.removeRows(startRow, count);
});

return queryObjectMatrix(matrix, (value) => value === 1);
};

export const runRefRangeMutations = (operators: IOperator[], range: IRange) => {
let result: Nullable<IRange> = { ...range };
operators.forEach((operator) => {
Expand Down Expand Up @@ -1032,14 +1051,27 @@ export const handleCommonDefaultRangeChangeWithEffectRefCommands = (range: IRang
break;
}
case EffectRefRangId.RemoveRowCommandId: {
operator = handleIRemoveRow(commandInfo as IRemoveRowColCommand, range);
break;
return handleRemoveRowCommon(commandInfo.params as IRemoveRowColCommandInterceptParams, range);
}
}
const resultRange = runRefRangeMutations(operator, range);
return resultRange;
};

export const handleCommonRangeChangeWithEffectRefCommandsSkipNoInterests = (range: IRange, commandInfo: ICommandInfo, deps: { selectionManagerService: SheetsSelectionsService }) => {
const skipCommands = [DeleteRangeMoveLeftCommand.id, DeleteRangeMoveUpCommand.id];
if (skipCommands.includes(commandInfo.id)) {
return handleCommonDefaultRangeChangeWithEffectRefCommands(range, commandInfo);
}

const effectRanges = getEffectedRangesOnCommand(commandInfo as EffectRefRangeParams, deps);
if (effectRanges.some((effectRange) => Rectangle.intersects(effectRange, range))) {
return handleCommonDefaultRangeChangeWithEffectRefCommands(range, commandInfo);
}

return range;
};

/**
* This function should work as a pure function.
*
Expand Down

0 comments on commit 014dd67

Please sign in to comment.