Skip to content

Commit

Permalink
#5618 ketcher doesnt trigger change event in macromolecule mode (#6012)
Browse files Browse the repository at this point in the history
* testing PR

* change history

* fix error in global d ts

* fix after meeting

* fix after push

* update test

* fix after review

* fix after review

* fix after review

* change update dispatch

* fix after test ci cd

* add comments

* fix after falied test

---------

Co-authored-by: Andrey Menshikov <[email protected]>
  • Loading branch information
Drimodaren and Andrey Menshikov authored Nov 29, 2024
1 parent 08e58c6 commit 4f28cec
Show file tree
Hide file tree
Showing 5 changed files with 191 additions and 164 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ test.describe('Track Changes', () => {
Test case: EPMLSOPKET-1989
Description: Add Nitrogen atom to canvas 35 times and then press Undo 32 times
*/
test.slow();

const atomType = AtomButton.Nitrogen;

Expand Down
7 changes: 4 additions & 3 deletions packages/ketcher-core/src/application/editor/EditorHistory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import { Command } from 'domain/entities/Command';
import { CoreEditor } from './Editor';
import assert from 'assert';
import { ketcherProvider } from 'application/utils';
const HISTORY_SIZE = 32; // put me to options

export type HistoryOperationType = 'undo' | 'redo';
Expand All @@ -35,7 +36,6 @@ export class EditorHistory {
this.historyPointer = 0;

EditorHistory._instance = this;

return this;
}

Expand All @@ -50,13 +50,14 @@ export class EditorHistory {
}
this.historyPointer = this.historyStack.length;
}
ketcherProvider.getKetcher()?.changeEvent.dispatch();
}

undo() {
if (this.historyPointer === 0) {
return;
}

ketcherProvider.getKetcher()?.changeEvent.dispatch();
assert(this.editor);

this.historyPointer--;
Expand All @@ -71,7 +72,7 @@ export class EditorHistory {
if (this.historyPointer === this.historyStack.length) {
return;
}

ketcherProvider.getKetcher()?.changeEvent.dispatch();
assert(this.editor);

const lastCommand = this.historyStack[this.historyPointer];
Expand Down
4 changes: 3 additions & 1 deletion packages/ketcher-core/src/application/ketcher.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Subscription } from 'subscription';
/****************************************************************************
* Copyright 2021 EPAM Systems
*
Expand Down Expand Up @@ -65,6 +66,7 @@ export class Ketcher {
#editor: Editor;
_indigo: Indigo;
#eventBus: EventEmitter;
changeEvent: Subscription;

get editor(): Editor {
return this.#editor;
Expand All @@ -82,7 +84,7 @@ export class Ketcher {
assert(editor != null);
assert(structService != null);
assert(formatterFactory != null);

this.changeEvent = new Subscription();
this.#editor = editor;
this.structService = structService;
this.#formatterFactory = formatterFactory;
Expand Down
22 changes: 15 additions & 7 deletions packages/ketcher-react/src/script/editor/Editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
Scale,
Struct,
Vec2,
ketcherProvider,
} from 'ketcher-core';
import {
DOMSubscription,
Expand Down Expand Up @@ -190,7 +191,6 @@ class Editor implements KetcherEditor {
this.renderAndRecoordinateStruct.bind(this);
this.setOptions = this.setOptions.bind(this);
this.setServerSettings(serverSettings);

this.lastCursorPosition = {
x: 0,
y: 0,
Expand Down Expand Up @@ -356,6 +356,7 @@ class Editor implements KetcherEditor {
// We need to reset the tool to make sure it was recreated
this.tool('select');
this.event.change.dispatch('force');
ketcherProvider.getKetcher().changeEvent.dispatch('force');
}
}

Expand Down Expand Up @@ -543,7 +544,8 @@ class Editor implements KetcherEditor {
this.historyStack.shift();
}
this.historyPtr = this.historyStack.length;
this.event.change.dispatch(action); // TODO: stoppable here
this.event.change.dispatch(action); // TODO: stoppable here. This has to be removed, however some implicit subscription to change event exists somewhere in the app and removing it leads to unexpected behavior, investigate further
ketcherProvider.getKetcher().changeEvent.dispatch(action);
}
this.render.update(false, null);
}
Expand All @@ -557,6 +559,7 @@ class Editor implements KetcherEditor {
}

undo() {
const ketcherChangeEvent = ketcherProvider.getKetcher().changeEvent;
if (this.historyPtr === 0) {
throw new Error('Undo stack is empty');
}
Expand All @@ -573,15 +576,18 @@ class Editor implements KetcherEditor {
this.historyStack[this.historyPtr] = action;

if (this._tool instanceof toolsMap.paste) {
this.event.change.dispatch();
this.event.change.dispatch(); // TODO: stoppable here. This has to be removed, however some implicit subscription to change event exists somewhere in the app and removing it leads to unexpected behavior, investigate further
ketcherChangeEvent.dispatch();
} else {
this.event.change.dispatch(action);
this.event.change.dispatch(action); // TODO: stoppable here. This has to be removed, however some implicit subscription to change event exists somewhere in the app and removing it leads to unexpected behavior, investigate further
ketcherChangeEvent.dispatch(action);
}

this.render.update();
}

redo() {
const ketcherChangeEvent = ketcherProvider.getKetcher().changeEvent;
if (this.historyPtr === this.historyStack.length) {
throw new Error('Redo stack is empty');
}
Expand All @@ -597,9 +603,11 @@ class Editor implements KetcherEditor {
this.historyPtr++;

if (this._tool instanceof toolsMap.paste) {
this.event.change.dispatch();
this.event.change.dispatch(); // TODO: stoppable here. This has to be removed, however some implicit subscription to change event exists somewhere in the app and removing it leads to unexpected behavior, investigate further
ketcherChangeEvent.dispatch();
} else {
this.event.change.dispatch(action);
this.event.change.dispatch(action); // TODO: stoppable here. This has to be removed, however some implicit subscription to change event exists somewhere in the app and removing it leads to unexpected behavior, investigate further
ketcherChangeEvent.dispatch(action);
}

this.render.update();
Expand All @@ -615,7 +623,7 @@ class Editor implements KetcherEditor {
const subscribeFuncWrapper = (action) =>
customOnChangeHandler(action, handler);
subscriber.handler = subscribeFuncWrapper;
this.event[eventName].add(subscribeFuncWrapper);
ketcherProvider.getKetcher().changeEvent.add(subscribeFuncWrapper);
break;
}

Expand Down
Loading

0 comments on commit 4f28cec

Please sign in to comment.