Skip to content

Commit

Permalink
fix: editing changing should not focus
Browse files Browse the repository at this point in the history
  • Loading branch information
@wzhudev
wzhudev committed Mar 11, 2024
1 parent d420655 commit 84d2853
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 37 deletions.
14 changes: 7 additions & 7 deletions packages/core/src/services/command/command.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export interface ICommand<P extends object = object, R = boolean> {
readonly id: string;
readonly type: CommandType;

handler(accessor: IAccessor, params?: P): Promise<R> | R;
handler(accessor: IAccessor, params?: P, options?: IExecutionOptions): Promise<R> | R;

/**
* When this command is unregistered, this function would be called.
Expand Down Expand Up @@ -259,7 +259,7 @@ export class CommandService implements ICommandService {
const stackItemDisposable = this._pushCommandExecutionStack(commandInfo);

this._beforeCommandExecutionListeners.forEach((listener) => listener(commandInfo, options));
const result = await this._execute<P, R>(command as ICommand<P, R>, params);
const result = await this._execute<P, R>(command as ICommand<P, R>, params, options);
this._commandExecutedListeners.forEach((listener) => listener(commandInfo, options));

stackItemDisposable.dispose();
Expand Down Expand Up @@ -300,7 +300,7 @@ export class CommandService implements ICommandService {
const stackItemDisposable = this._pushCommandExecutionStack(commandInfo);

this._beforeCommandExecutionListeners.forEach((listener) => listener(commandInfo, options));
const result = this._syncExecute<P, R>(command as ICommand<P, R>, params);
const result = this._syncExecute<P, R>(command as ICommand<P, R>, params, options);
this._commandExecutedListeners.forEach((listener) => listener(commandInfo, options));

stackItemDisposable.dispose();
Expand Down Expand Up @@ -353,7 +353,7 @@ export class CommandService implements ICommandService {
});
}

private async _execute<P extends object, R = boolean>(command: ICommand<P, R>, params?: P): Promise<R> {
private async _execute<P extends object, R = boolean>(command: ICommand<P, R>, params?: P, options?: IExecutionOptions): Promise<R> {
this._logService.debug(
'[CommandService]',
`${'|-'.repeat(Math.max(this._commandExecutingLevel, 0))}executing command "${command.id}"`
Expand All @@ -362,7 +362,7 @@ export class CommandService implements ICommandService {
this._commandExecutingLevel++;
let result: R | boolean;
try {
result = await this._injector.invoke(command.handler, params);
result = await this._injector.invoke(command.handler, params, options);
this._commandExecutingLevel--;
} catch (e) {
result = false;
Expand All @@ -373,7 +373,7 @@ export class CommandService implements ICommandService {
return result;
}

private _syncExecute<P extends object, R = boolean>(command: ICommand<P, R>, params?: P): R {
private _syncExecute<P extends object, R = boolean>(command: ICommand<P, R>, params?: P, options?: IExecutionOptions): R {
this._logService.debug(
'[CommandService]',
`${'|-'.repeat(Math.max(0, this._commandExecutingLevel))}executing command "${command.id}".`
Expand All @@ -382,7 +382,7 @@ export class CommandService implements ICommandService {
this._commandExecutingLevel++;
let result: R | boolean;
try {
result = this._injector.invoke(command.handler, params) as R;
result = this._injector.invoke(command.handler, params, options) as R;
if (result instanceof Promise) {
throw new TypeError('[CommandService]: Command handler should not return a promise.');
}
Expand Down
14 changes: 10 additions & 4 deletions packages/find-replace/src/services/find-replace.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ export interface IFindMoveParams {

/** If the the selection is on the match and then should stay on the match. */
stayIfOnMatch?: boolean;

/**
* If this param is true, we should only change matching position without performing focusing.
* This usually happens when "moving" is triggered when a document's content changes.
*/
noFocus?: boolean;
}

export interface IReplaceAllResult {
Expand Down Expand Up @@ -313,7 +319,7 @@ export class FindReplaceModel extends Disposable {
.subscribe(([...allMatches]) => {
const newMatches = this._matches = allMatches.flat();
if (newMatches.length) {
const index = this._moveToInitialMatch(this._findModels, newMatches);
const index = this._moveToInitialMatch(this._findModels, newMatches, true);
this._state.changeState({ matchesCount: newMatches.length, matchesPosition: index + 1, findCompleted: false });
this.replaceables$.next(newMatches.filter((m) => m.replaceable) as IReplaceableMatch[]);
} else {
Expand Down Expand Up @@ -411,7 +417,7 @@ export class FindReplaceModel extends Disposable {
}
}

private _moveToInitialMatch(findModels: FindModel[], results: IFindMatch[]): number {
private _moveToInitialMatch(findModels: FindModel[], results: IFindMatch[], noFocus = false): number {
const focusedUnitId = this._univerInstanceService.getFocusedUniverInstance()?.getUnitId();
if (!focusedUnitId) {
return -1;
Expand All @@ -420,15 +426,15 @@ export class FindReplaceModel extends Disposable {
const findModelOnFocusUnit = findModels.find((model) => model.unitId === focusedUnitId);
if (findModelOnFocusUnit) {
this._matchingModel = findModelOnFocusUnit;
const nextMatch = findModelOnFocusUnit.moveToNextMatch({ stayIfOnMatch: true });
const nextMatch = findModelOnFocusUnit.moveToNextMatch({ stayIfOnMatch: true, noFocus });
const index = results.findIndex((value) => value === nextMatch);
this.currentMatch$.next(nextMatch);
return index;
}

// otherwise we just simply match the first result
this._matchingModel = findModels[0];
const nextMatch = this._matchingModel.moveToNextMatch();
const nextMatch = this._matchingModel.moveToNextMatch({ noFocus });
const matchPosition = this._matches.findIndex((m) => m === nextMatch);
this.currentMatch$.next(nextMatch);
return matchPosition;
Expand Down
59 changes: 36 additions & 23 deletions packages/sheets-find-replace/src/controllers/sheet-find-replace.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ import { IRenderManagerService, RENDER_RAW_FORMULA_KEY } from '@univerjs/engine-
import type { IFindComplete, IFindMatch, IFindMoveParams, IFindQuery, IFindReplaceProvider, IReplaceAllResult } from '@univerjs/find-replace';
import { FindBy, FindDirection, FindModel, FindScope, IFindReplaceService } from '@univerjs/find-replace';
import type { ISetRangeValuesCommandParams, ISetWorksheetActivateCommandParams, ISheetCommandSharedParams } from '@univerjs/sheets';
import { SelectionManagerService, SetRangeValuesCommand, SetWorksheetActivateCommand } from '@univerjs/sheets';
import { SelectionManagerService, SetRangeValuesCommand, SetWorksheetActivateCommand, SetWorksheetActiveOperation } from '@univerjs/sheets';
import type { IScrollToCellCommandParams } from '@univerjs/sheets-ui';
import { getCoordByCell, getSheetObject, ScrollToCellCommand, SheetSkeletonManagerService } from '@univerjs/sheets-ui';
import { type IDisposable, Inject, Injector } from '@wendellhu/redi';
import { deserializeRangeWithSheet } from '@univerjs/engine-formula';
import { delay, filter, merge, skip, Subject, throttleTime } from 'rxjs';
import { filter, merge, skip, Subject, throttleTime } from 'rxjs';

import type { ISheetFindReplaceHighlightShapeProps } from '../views/shapes/find-replace-highlight.shape';
import { SheetFindReplaceHighlightShape } from '../views/shapes/find-replace-highlight.shape';
Expand Down Expand Up @@ -217,19 +217,21 @@ export class SheetFindModel extends FindModel {
this._updateCurrentHighlightShape(this._activeHighlightIndex);
}));

this.disposeWithMe(this._workbook.activeSheet$.pipe(skip(1), delay(200)).subscribe((activeSheet) => {
if (!activeSheet) {
return;
}

// If there's no match in the new worksheet, it wouldn't change matching position.
const activeSheetId = activeSheet.getSheetId();
if (!this._matchesByWorksheet.has(activeSheetId)) {
return;
}
this.disposeWithMe(
fromCallback(this._commandService.onCommandExecuted)
.pipe(
filter(([command, options]) => command.id === SetWorksheetActiveOperation.id && !options?.fromFindReplace)
)
.subscribe(() => {
const activeSheet = this._workbook.getActiveSheet();
const activeSheetId = activeSheet.getSheetId();
if (!this._matchesByWorksheet.has(activeSheetId)) {
return;
}

this._findNextMatchOnActiveSheetChange(activeSheet);
}));
this._findNextMatchOnActiveSheetChange(activeSheet);
})
);

// When the sheet model changes, we should re-search.
this.disposeWithMe(
Expand Down Expand Up @@ -542,13 +544,17 @@ export class SheetFindModel extends FindModel {
private _focusMatch(match: ISheetCellMatch): void {
const subUnitId = match.range.subUnitId;
if (subUnitId !== this._workbook.getActiveSheet().getSheetId()) {
this._commandService.syncExecuteCommand(SetWorksheetActivateCommand.id, {
unitId: this._workbook.getUnitId(),
subUnitId,
} as ISetWorksheetActivateCommandParams);
this._commandService.syncExecuteCommand(SetWorksheetActivateCommand.id,
{ unitId: this._workbook.getUnitId(), subUnitId } as ISetWorksheetActivateCommandParams,
{ fromFindReplace: true }
);
}

this._commandService.syncExecuteCommand(ScrollToCellCommand.id, { range: match.range.range } as IScrollToCellCommandParams);
this._commandService.syncExecuteCommand(
ScrollToCellCommand.id,
{ range: match.range.range } as IScrollToCellCommandParams,
{ fromFindReplace: true }
);
}

private _tryRestoreLastMatchesPosition(lastMatch: Nullable<ISheetCellMatch>, newMatches: ISheetCellMatch[]): number {
Expand All @@ -575,6 +581,7 @@ export class SheetFindModel extends FindModel {

const loop = params?.loop ?? false;
const stayIfOnMatch = params?.stayIfOnMatch ?? false;
const noFocus = params?.noFocus ?? false;

const matchToMove = this._findNextMatch(loop, stayIfOnMatch);
if (matchToMove) {
Expand All @@ -587,8 +594,11 @@ export class SheetFindModel extends FindModel {
this._activeHighlightIndex = index;
}

this._focusMatch(match);
this._updateCurrentHighlightShape(this._activeHighlightIndex);
if (!noFocus) {
this._focusMatch(match);
this._updateCurrentHighlightShape(this._activeHighlightIndex);
}

return match;
}

Expand All @@ -604,6 +614,7 @@ export class SheetFindModel extends FindModel {

const loop = params?.loop ?? false;
const stayIfOnMatch = params?.stayIfOnMatch ?? false;
const noFocus = params?.noFocus ?? false;

const matchToMove = this._findPreviousMatch(loop, stayIfOnMatch);
if (matchToMove) {
Expand All @@ -616,8 +627,10 @@ export class SheetFindModel extends FindModel {
this._activeHighlightIndex = index;
}

this._focusMatch(match);
this._updateCurrentHighlightShape(this._activeHighlightIndex);
if (!noFocus) {
this._focusMatch(match);
this._updateCurrentHighlightShape(this._activeHighlightIndex);
}
return match;
}

Expand Down
6 changes: 3 additions & 3 deletions packages/sheets/src/commands/commands/set-worksheet-activate.command.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import type { ICommand } from '@univerjs/core';
import type { ICommand, IExecutionOptions } from '@univerjs/core';
import { CommandType, ICommandService, IUniverInstanceService } from '@univerjs/core';
import type { IAccessor } from '@wendellhu/redi';

Expand All @@ -29,7 +29,7 @@ export interface ISetWorksheetActivateCommandParams {
export const SetWorksheetActivateCommand: ICommand = {
type: CommandType.COMMAND,
id: 'sheet.command.set-worksheet-activate',
handler: (accessor: IAccessor, params?: ISetWorksheetActivateCommandParams) => {
handler: (accessor: IAccessor, params?: ISetWorksheetActivateCommandParams, options?: IExecutionOptions) => {
const commandService = accessor.get(ICommandService);
const univerInstanceService = accessor.get(IUniverInstanceService);

Expand All @@ -44,6 +44,6 @@ export const SetWorksheetActivateCommand: ICommand = {
return commandService.syncExecuteCommand(SetWorksheetActiveOperation.id, {
unitId,
subUnitId,
} as ISetWorksheetActiveOperationParams);
} as ISetWorksheetActiveOperationParams, options);
},
};

0 comments on commit 84d2853

Please sign in to comment.