(core) Disable undo/redo in detached formula editor

Summary:
Undo and redo were causing errors to be thrown while the editor was detached. In the
interest of time, we'll disable undo/redo until we have a chance to look at whether
we can support it in the editor.

Test Plan: Manual.

Reviewers: JakubSerafin

Reviewed By: JakubSerafin

Differential Revision: https://phab.getgrist.com/D3959
This commit is contained in:
George Gevoian 2023-07-18 22:27:53 -04:00
parent c0ed4a8a60
commit 0040716006
3 changed files with 43 additions and 8 deletions

View File

@ -25,7 +25,7 @@ export interface IUndoState {
* position in this stack. Undo and redo actions are generated and sent to the server here. * position in this stack. Undo and redo actions are generated and sent to the server here.
*/ */
export class UndoStack extends dispose.Disposable { export class UndoStack extends dispose.Disposable {
public isDisabled: Observable<boolean>;
public undoDisabledObs: ko.Observable<boolean>; public undoDisabledObs: ko.Observable<boolean>;
public redoDisabledObs: ko.Observable<boolean>; public redoDisabledObs: ko.Observable<boolean>;
private _gristDoc: GristDoc; private _gristDoc: GristDoc;
@ -40,6 +40,8 @@ export class UndoStack extends dispose.Disposable {
public create(log: MinimalActionGroup[], options: {gristDoc: GristDoc}) { public create(log: MinimalActionGroup[], options: {gristDoc: GristDoc}) {
this._gristDoc = options.gristDoc; this._gristDoc = options.gristDoc;
this.isDisabled = Observable.create(this, false);
// TODO: _stack and _linkMap grow without bound within a single session. // TODO: _stack and _linkMap grow without bound within a single session.
// The top of the stack is stack.length - 1. The pointer points above the most // The top of the stack is stack.length - 1. The pointer points above the most
// recently applied (not undone) action. // recently applied (not undone) action.
@ -99,15 +101,27 @@ export class UndoStack extends dispose.Disposable {
} }
// Send an undo action. This should be called when the user presses 'undo'. // Send an undo action. This should be called when the user presses 'undo'.
public sendUndoAction(): Promise<void> { public async sendUndoAction(): Promise<void> {
if (this.isDisabled.get()) { return; }
return this._undoChain.add(() => this._sendAction(true)); return this._undoChain.add(() => this._sendAction(true));
} }
// Send a redo action. This should be called when the user presses 'redo'. // Send a redo action. This should be called when the user presses 'redo'.
public sendRedoAction(): Promise<void> { public async sendRedoAction(): Promise<void> {
if (this.isDisabled.get()) { return; }
return this._undoChain.add(() => this._sendAction(false)); return this._undoChain.add(() => this._sendAction(false));
} }
public enable(): void {
this.isDisabled.set(false);
}
public disable(): void {
this.isDisabled.set(true);
}
private async _sendAction(isUndo: boolean): Promise<void> { private async _sendAction(isUndo: boolean): Promise<void> {
// Pick the action group to undo or redo. // Pick the action group to undo or redo.
const ag = this._stack[isUndo ? this._pointer - 1 : this._pointer]; const ag = this._stack[isUndo ? this._pointer - 1 : this._pointer];

View File

@ -70,6 +70,14 @@ export function createTopBarDoc(owner: MultiHolder, appModel: AppModel, pageMode
return module.SearchModelImpl.create(use.owner, gristDoc); return module.SearchModelImpl.create(use.owner, gristDoc);
}); });
const isUndoRedoAvailable = Computed.create(owner, use => {
const gristDoc = use(pageModel.gristDoc);
if (!gristDoc) { return false; }
const undoStack = gristDoc.getUndoStack();
return !use(undoStack.isDisabled);
});
return [ return [
// TODO Before gristDoc is loaded, we could show doc-name without the page. For now, we delay // TODO Before gristDoc is loaded, we could show doc-name without the page. For now, we delay
// showing of breadcrumbs until gristDoc is loaded. // showing of breadcrumbs until gristDoc is loaded.
@ -98,14 +106,14 @@ export function createTopBarDoc(owner: MultiHolder, appModel: AppModel, pageMode
topBarUndoBtn('Undo', topBarUndoBtn('Undo',
dom.on('click', () => state.isUndoDisabled.get() || allCommands.undo.run()), dom.on('click', () => state.isUndoDisabled.get() || allCommands.undo.run()),
hoverTooltip('Undo', {key: 'topBarBtnTooltip'}), hoverTooltip('Undo', {key: 'topBarBtnTooltip'}),
cssHoverCircle.cls('-disabled', state.isUndoDisabled), cssHoverCircle.cls('-disabled', use => use(state.isUndoDisabled) || !use(isUndoRedoAvailable)),
testId('undo') testId('undo'),
), ),
topBarUndoBtn('Redo', topBarUndoBtn('Redo',
dom.on('click', () => state.isRedoDisabled.get() || allCommands.redo.run()), dom.on('click', () => state.isRedoDisabled.get() || allCommands.redo.run()),
hoverTooltip('Redo', {key: 'topBarBtnTooltip'}), hoverTooltip('Redo', {key: 'topBarBtnTooltip'}),
cssHoverCircle.cls('-disabled', state.isRedoDisabled), cssHoverCircle.cls('-disabled', use => use(state.isRedoDisabled) || !use(isUndoRedoAvailable)),
testId('redo') testId('redo'),
), ),
cssSpacer(), cssSpacer(),
]), ]),

View File

@ -153,7 +153,20 @@ export class FormulaEditor extends NewBaseEditor {
this._isEmpty.addListener(() => this._updateEditorPlaceholder()); this._isEmpty.addListener(() => this._updateEditorPlaceholder());
// Update the placeholder text when expanding or collapsing the editor. // Update the placeholder text when expanding or collapsing the editor.
this.isDetached.addListener(() => this._updateEditorPlaceholder()); this.isDetached.addListener((isDetached) => {
// TODO: look into whether we can support undo/redo while the editor is detached.
if (isDetached) {
options.gristDoc.getUndoStack().disable();
} else {
options.gristDoc.getUndoStack().enable();
}
this._updateEditorPlaceholder();
});
this.onDispose(() => {
options.gristDoc.getUndoStack().enable();
});
this._dom = cssFormulaEditor( this._dom = cssFormulaEditor(
// switch border shadow // switch border shadow