Skip to content

Commit

Permalink
fix: add sheetId when press arrow key in prompt for ref selection.
Browse files Browse the repository at this point in the history 
chore: test
  • Loading branch information
@lumixraku
lumixraku committed Dec 2, 2024
1 parent 106f147 commit f8671f1
Show file tree
Hide file tree
Showing 12 changed files with 102 additions and 82 deletions.
12 changes: 9 additions & 3 deletions packages/sheets-formula-ui/src/controllers/prompt.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -906,7 +906,7 @@ export class PromptController extends Disposable {

const bodyList = this._getFormulaAndCellEditorBody(unitIds).filter((b) => !!b);

this._refSelectionsService.clear();
// this._refSelectionsService.clear();

if (sequenceNodes == null || sequenceNodes.length === 0) {
this._existsSequenceNode = false;
Expand Down Expand Up @@ -1073,7 +1073,7 @@ export class PromptController extends Disposable {
// }

if (selectionWithStyle.length) {
this._refSelectionsService.addSelections(unitId, currSheetId, selectionWithStyle);
this._refSelectionsService.setSelections(unitId, currSheetId, selectionWithStyle);
}
}

Expand Down Expand Up @@ -1405,6 +1405,12 @@ export class PromptController extends Disposable {
return;
}

const { skeleton } = this._getCurrentUnitIdAndSheetId();
const unitId = skeleton?.worksheet.getUnitId();
const sheetId = skeleton?.worksheet.getSheetId();
currentSelection.range.sheetId = sheetId;
currentSelection.range.unitId = unitId;

const refString = this._generateRefString(currentSelection);
this._formulaPromptService.setSequenceNodes(insertNodes);
this._formulaPromptService.insertSequenceRef(this._previousInsertRefStringIndex, refString);
Expand Down Expand Up @@ -1830,7 +1836,7 @@ export class PromptController extends Disposable {
const selectionData = this._sheetsSelectionsService.getCurrentLastSelection();
if (selectionData != null) {
const selectionDataNew = Tools.deepClone(selectionData);
this._refSelectionsService.addSelections([selectionDataNew]);
this._refSelectionsService.setSelections([selectionDataNew]);
}
}

Expand Down
29 changes: 12 additions & 17 deletions packages/sheets-formula-ui/src/services/render-services/ref-selections.render-service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,17 @@ export class RefSelectionsRenderService extends BaseSelectionRenderService imple
return control;
}

private _initSelectionChangeListener(): void {
// used for refresh selection when focus in formula editor.
this.disposeWithMe(this._refSelectionsService.selectionSet$.subscribe((selectionsWithStyles) => {
this._reset();
const skeleton = this._skeleton;
if (!skeleton) return;
// The selections' style would be colorful here. PromptController would change the color of selections later.
this.resetSelectionsByModelData(selectionsWithStyles || []);
}));
}

/**
* Update selectionModel in this._workbookSelections by user action in spreadsheet area.
*/
Expand Down Expand Up @@ -205,22 +216,6 @@ export class RefSelectionsRenderService extends BaseSelectionRenderService imple
);
}

private _initSelectionChangeListener(): void {
// selectionMoveEnd$ beforeSelectionMoveEnd$ was triggered when pointerup after dragging to change selection area.
// Changing the selection area through the 8 control points of the ref selection will not trigger this subscriber.

// beforeSelectionMoveEnd$ & selectionMoveEnd$ would triggered when change skeleton(change sheet).
this.disposeWithMe(this._workbookSelections.selectionSet$.subscribe((selectionsWithStyles) => {
this._reset();
const skeleton = this._skeleton;
if (!skeleton) return;
// The selections' style would be colorful here. PromptController would change the color of selections later.
for (const selectionWithStyle of selectionsWithStyles) {
this._addSelectionControlByModelData(selectionWithStyle);
}
}));
}

private _initSkeletonChangeListener(): void {
// changing sheet is not the only way cause currentSkeleton$ emit, a lot of cmds will emit currentSkeleton$
// COMMAND_LISTENER_SKELETON_CHANGE ---> currentSkeleton$.next
Expand Down Expand Up @@ -266,7 +261,7 @@ export class RefSelectionsRenderService extends BaseSelectionRenderService imple
* @param viewport
* @param scrollTimerType
*/
// eslint-disable-next-line max-lines-per-function, complexity
// eslint-disable-next-line complexity
protected _onPointerDown(
evt: IPointerEvent | IMouseEvent,
_zIndex = 0,
Expand Down
9 changes: 5 additions & 4 deletions packages/sheets-ui/src/commands/commands/auto-fill.command.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import type { IAccessor, ICommand, IRange } from '@univerjs/core';
import type { ISetRangeValuesMutationParams } from '@univerjs/sheets';
import type { ISetRangeValuesMutationParams, ISetSelectionsOperationParams } from '@univerjs/sheets';
import { CommandType, ICommandService, IUndoRedoService, IUniverInstanceService, sequenceExecute } from '@univerjs/core';
import { generateNullCellValue, getSheetCommandTarget, SetRangeValuesMutation, SetRangeValuesUndoMutationFactory, SetSelectionsOperation, SheetInterceptorService } from '@univerjs/sheets';

Expand Down Expand Up @@ -66,7 +66,7 @@ export const AutoClearContentCommand: ICommand = {
clearMutationParams
);
const { startColumn, startRow } = selectionRange;
commandService.executeCommand(SetSelectionsOperation.id, {
const param: ISetSelectionsOperationParams = {
selections: [
{
primary: {
Expand All @@ -76,7 +76,7 @@ export const AutoClearContentCommand: ICommand = {
endRow: startRow,
actualRow: startRow,
actualColumn: startColumn,
isMerge: false,
isMerged: false,
isMergedMainCell: false,
},
range: {
Expand All @@ -86,7 +86,8 @@ export const AutoClearContentCommand: ICommand = {
],
unitId,
subUnitId,
});
};
commandService.executeCommand(SetSelectionsOperation.id, param);

const result = commandService.syncExecuteCommand(SetRangeValuesMutation.id, clearMutationParams);
if (result) {
Expand Down
14 changes: 7 additions & 7 deletions packages/sheets-ui/src/commands/commands/set-selection.command.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,17 @@
*/

import type { Direction, ICommand, IRange } from '@univerjs/core';
import type {
ISetSelectionsOperationParams } from '@univerjs/sheets';
import { CommandType, ICommandService, IUniverInstanceService, RANGE_TYPE, Rectangle, Tools } from '@univerjs/core';
import { IRenderManagerService } from '@univerjs/engine-render';

import { IRenderManagerService } from '@univerjs/engine-render';
import {
expandToContinuousRange,
getCellAtRowCol,
getSelectionsService,
getSheetCommandTarget,
SelectionMoveType,
SetSelectionsOperation,
} from '@univerjs/sheets';
import { KeyCode } from '@univerjs/ui';
Expand Down Expand Up @@ -62,7 +65,7 @@ export interface IMoveSelectionEnterAndTabCommandParams {
}

/**
* Move selection (Mainly by keyboard arrow keys, Tab and Enter key see MoveSelectionEnterAndTabCommand)
* Move selection (Mainly by keyboard arrow keys, For Tab and Enter key, check @MoveSelectionEnterAndTabCommand)
*/
export const MoveSelectionCommand: ICommand<IMoveSelectionCommandParams> = {
id: 'sheet.command.move-selection',
Expand Down Expand Up @@ -125,10 +128,8 @@ export const MoveSelectionCommand: ICommand<IMoveSelectionCommandParams> = {
unitId: workbook.getUnitId(),
subUnitId: worksheet.getSheetId(),
selections,
});
const renderManagerService = accessor.get(IRenderManagerService);
const selectionService = renderManagerService.getRenderById(unitId)?.with(ISheetSelectionRenderService);
selectionService?.refreshSelectionMoveEnd();
type: SelectionMoveType.MOVE_END,
} as ISetSelectionsOperationParams);
return rs;
},
};
Expand Down Expand Up @@ -270,7 +271,6 @@ export const MoveSelectionEnterAndTabCommand: ICommand<IMoveSelectionEnterAndTab
const rs = accessor.get(ICommandService).executeCommand(SetSelectionsOperation.id, {
unitId,
subUnitId: sheetId,

selections: [resultRange],
});
const renderManagerService = accessor.get(IRenderManagerService);
Expand Down
1 change: 0 additions & 1 deletion packages/sheets-ui/src/controllers/menu/permission-menu-util.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -502,4 +502,3 @@ export function getViewPermissionDisable$(accessor: IAccessor) {
})
);
}

7 changes: 3 additions & 4 deletions packages/sheets-ui/src/services/selection/base-selection-render.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,13 +160,12 @@ export class BaseSelectionRenderService extends Disposable implements ISheetSele
readonly selectionMoveEnd$ = this._selectionMoveEnd$.asObservable();
protected readonly _selectionMoving$ = new Subject<ISelectionWithCoord[]>();
readonly selectionMoving$ = this._selectionMoving$.asObservable();

/**
* Mainly emit by pointerdown
*/
protected readonly _selectionMoveStart$ = new Subject<ISelectionWithCoord[]>();
readonly selectionMoveStart$ = this._selectionMoveStart$.asObservable();

/**
* Mainly emit by pointermove in spreadsheet
*/
private _selectionMoving = false;
get selectionMoving(): boolean {
return this._selectionMoving;
Expand Down
28 changes: 20 additions & 8 deletions packages/sheets-ui/src/services/selection/selection-render.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export class SheetSelectionRenderService extends BaseSelectionRenderService impl
const sheetObject = this._getSheetObject();

this._initEventListeners(sheetObject);
this._initSelectionChangeListener();
this._initSelectionModelChangeListener();
this._initThemeChangeListener();
this._initSkeletonChangeListener();
this._initUserActionSyncListener();
Expand Down Expand Up @@ -144,13 +144,21 @@ export class SheetSelectionRenderService extends BaseSelectionRenderService impl
return this._contextService.getContextValue(DISABLE_NORMAL_SELECTIONS);
}

private _initSelectionChangeListener(): void {
// normal selection: after dragging selection(move end)
this.disposeWithMe(merge(this._workbookSelections.selectionMoveEnd$, this._workbookSelections.selectionSet$).subscribe((selectionWithStyleList) => {
/**
* Response for selection model changing.
*/
private _initSelectionModelChangeListener(): void {
this.disposeWithMe(merge(
this._workbookSelections.selectionMoveEnd$, // triggered by keyboard(e.g. arrow key tab enter)
this._workbookSelections.selectionSet$ // shift + arrow key
).subscribe((selectionWithStyleList) => {
this.resetSelectionsByModelData(selectionWithStyleList);
}));
}

/**
* Handle events in spreadsheet. (e.g. drag and move to make a selection)
*/
private _initUserActionSyncListener(): void {
this.disposeWithMe(this.selectionMoveStart$.subscribe((params) => this._updateSelections(params, SelectionMoveType.MOVE_START)));
this.disposeWithMe(this.selectionMoving$.subscribe((params) => this._updateSelections(params, SelectionMoveType.MOVING)));
Expand All @@ -164,7 +172,9 @@ export class SheetSelectionRenderService extends BaseSelectionRenderService impl
this._reset();
} else {
this._renderDisposable = toDisposable(
this.selectionMoveEnd$.subscribe((params) => this._updateSelections(params, SelectionMoveType.MOVE_END))
this.selectionMoveEnd$.subscribe((params) => {
this._updateSelections(params, SelectionMoveType.MOVE_END)

Check failure on line 176 in packages/sheets-ui/src/services/selection/selection-render.service.ts

View workflow job for this annotation

GitHub Actions / eslint 

Missing semicolon
})
);
}
}));
Expand All @@ -184,13 +194,15 @@ export class SheetSelectionRenderService extends BaseSelectionRenderService impl
return;
}

const selectionWithStyles = selectionDataWithStyleList.map((selectionDataWithStyle) =>
convertSelectionDataToRange(selectionDataWithStyle)
);

this._commandService.executeCommand(SetSelectionsOperation.id, {
unitId,
subUnitId: sheetId,
type,
selections: selectionDataWithStyleList.map((selectionDataWithStyle) =>
convertSelectionDataToRange(selectionDataWithStyle)
),
selections: selectionWithStyles

Check failure on line 205 in packages/sheets-ui/src/services/selection/selection-render.service.ts

View workflow job for this annotation

GitHub Actions / eslint 

Missing trailing comma
});
}

Expand Down
10 changes: 3 additions & 7 deletions packages/sheets-ui/src/services/selection/selection-shape-extension.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { SELECTION_CONTROL_BORDER_BUFFER_WIDTH } from '@univerjs/sheets';
import { SheetSkeletonManagerService } from '../sheet-skeleton-manager.service';
import { ISheetSelectionRenderService } from './base-selection-render.service';
import { genNormalSelectionStyle, RANGE_FILL_PERMISSION_CHECK, RANGE_MOVE_PERMISSION_CHECK } from './const';
import { attachSelectionWithCoord } from './util';
import { attachPrimaryWithCoord, attachSelectionWithCoord } from './util';

const HELPER_SELECTION_TEMP_NAME = '__SpreadsheetHelperSelectionTempRect';

Expand Down Expand Up @@ -258,12 +258,8 @@ export class SelectionShapeExtension {
});

this._targetSelection = { ...selectionWithCoord.rangeWithCoord };
// DO NOT UPDATE CURR CELL while dragging whole selection.
// Updating the primary cell during the middle of a drag operation may result in the primary cell being out of range in certain scenarios.
// ex: dragging normal selection to a merged area. there is a check to see if this move is valid, if not, the selection process would revert back to original state.

// normal selection should keep the original state when dragging whole selection.
// Now ref selection needs _control.selectionMoving$ update selection when dragging.
const primaryWithCoordAndMergeInfo = attachPrimaryWithCoord(this._skeleton, primaryCell);
this._control.updateCurrCell(primaryWithCoordAndMergeInfo);
this._control.selectionMoving$.next(selectionWithCoord.rangeWithCoord);
}

Expand Down
2 changes: 1 addition & 1 deletion packages/sheets/src/basics/selection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ export interface ISelectionWithStyle extends ISelection {
* if primary is not defined, means not keep current primary cell.
*/
primary: Nullable<ISelectionCell>;
style: Nullable<Partial<ISelectionStyle>>;
style?: Nullable<Partial<ISelectionStyle>>;
}

export interface ISheetRangeLocation {
Expand Down
1 change: 1 addition & 0 deletions packages/sheets/src/services/selections/ref-selections.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export class RefSelectionsService extends SheetsSelectionsService {
this.selectionMoveStart$ = $.pipe(switchMap((ss) => merge(...ss.map((s) => s.selectionMoveStart$))));
this.selectionMoving$ = $.pipe(switchMap((ss) => merge(...ss.map((s) => s.selectionMoving$))));
this.selectionMoveEnd$ = $.pipe(switchMap((ss) => merge(...ss.map((s) => s.selectionMoveEnd$))));
this.selectionSet$ = $.pipe(switchMap((ss) => merge(...ss.map((s) => s.selectionSet$))));
}

private _getAliveWorkbooks$(): Observable<WorkbookSelectionModel[]> {
Expand Down
47 changes: 19 additions & 28 deletions packages/sheets/src/services/selections/selection-data-model.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,10 @@ export class WorkbookSelectionModel extends Disposable {
this._selectionSet$.complete();
}

/** Clear all selections in this workbook. */
clear(): void {
this._worksheetSelections.clear();
this._eventAfterSetSelections([]);
}

addSelections(sheetId: string, selectionDatas: ISelectionWithStyle[]): void {
const selections = this.getSelectionsOfWorksheet(sheetId);
selections.push(...selectionDatas);
this._eventAfterSetSelections(selections);
this._selectionSet$.next(selections);
}

/**
Expand All @@ -84,8 +78,8 @@ export class WorkbookSelectionModel extends Disposable {
// selectionDatas should not be same variable as this._worksheetSelections !!!
// but there are some place get selection from this._worksheetSelections and set selectionDatas(2nd parameter of this function ) cause selectionDatas is always []
// see univer/pull/2909
this.deleteSheetSelection(sheetId);
this.getSelectionsOfWorksheet(sheetId).push(...selectionDatas);
// this.deleteSheetSelection(sheetId);
this.setSelectionsOfWorksheet(sheetId, selectionDatas);

switch (type) {
case SelectionMoveType.MOVE_START:
Expand All @@ -99,11 +93,11 @@ export class WorkbookSelectionModel extends Disposable {
this._selectionMoveEnd$.next(selectionDatas);
break;
case SelectionMoveType.ONLY_SET: {
this._eventAfterSetSelections(selectionDatas);
this._selectionSet$.next(selectionDatas);
break;
}
default:
this._eventAfterSetSelections(selectionDatas);
this._selectionSet$.next(selectionDatas);
break;
}
}
Expand All @@ -129,30 +123,27 @@ export class WorkbookSelectionModel extends Disposable {
return this._worksheetSelections.get(sheetId)!;
}

private _getCurrentSelections(): ISelectionWithStyle[] {
return this.getSelectionsOfWorksheet(this._workbook.getActiveSheet()!.getSheetId());
setSelectionsOfWorksheet(sheetId: string, selections: ISelectionWithStyle[]): void {
this._worksheetSelections.set(sheetId, [...selections]);
}

getCurrentLastSelection(): Readonly<Nullable<ISelectionWithStyle & { primary: ISelectionCell }>> {
const selectionData = this._getCurrentSelections();
return selectionData[selectionData.length - 1] as Readonly<Nullable<ISelectionWithStyle & { primary: ISelectionCell }>>;
deleteSheetSelection(sheetId: string) {
this._worksheetSelections.set(sheetId, []);
}

/**
* Same method as getSelectionsOfWorksheet.
* @param sheetId
* @returns this._worksheetSelections
*/
private _ensureSheetSelection(sheetId: string): ISelectionWithStyle[] {
if (!this._worksheetSelections.get(sheetId)) {
this._worksheetSelections.set(sheetId, []);
}
/** Clear all selections in this workbook. */
clear(): void {
this._worksheetSelections.clear();
this._eventAfterSetSelections([]);
}

return this._worksheetSelections.get(sheetId)!;
private _getCurrentSelections(): ISelectionWithStyle[] {
return this.getSelectionsOfWorksheet(this._workbook.getActiveSheet()!.getSheetId());
}

deleteSheetSelection(sheetId: string) {
this._worksheetSelections.set(sheetId, []);
getCurrentLastSelection(): Readonly<Nullable<ISelectionWithStyle & { primary: ISelectionCell }>> {
const selectionData = this._getCurrentSelections();
return selectionData[selectionData.length - 1] as Readonly<Nullable<ISelectionWithStyle & { primary: ISelectionCell }>>;
}

private _eventAfterSetSelections(selections: ISelectionWithStyle[]): void {
Expand Down
Loading

0 comments on commit f8671f1

Please sign in to comment.