From 4f28ceca256f8b2da5e0695955407e2f1a356dd8 Mon Sep 17 00:00:00 2001 From: Drimodaren <95479023+Drimodaren@users.noreply.github.com> Date: Fri, 29 Nov 2024 14:24:58 +0400 Subject: [PATCH] #5618 ketcher doesnt trigger change event in macromolecule mode (#6012) * 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 --- .../Track-Changes/track-changes.spec.ts | 1 + .../src/application/editor/EditorHistory.ts | 7 +- .../ketcher-core/src/application/ketcher.ts | 4 +- .../ketcher-react/src/script/editor/Editor.ts | 22 +- .../editor/utils/customOnChangeHandler.ts | 321 +++++++++--------- 5 files changed, 191 insertions(+), 164 deletions(-) diff --git a/ketcher-autotests/tests/Structure-Creating-&-Editing/Actions-With-Structures/Track-Changes/track-changes.spec.ts b/ketcher-autotests/tests/Structure-Creating-&-Editing/Actions-With-Structures/Track-Changes/track-changes.spec.ts index 11d23ab884..5214c9ebcc 100644 --- a/ketcher-autotests/tests/Structure-Creating-&-Editing/Actions-With-Structures/Track-Changes/track-changes.spec.ts +++ b/ketcher-autotests/tests/Structure-Creating-&-Editing/Actions-With-Structures/Track-Changes/track-changes.spec.ts @@ -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; diff --git a/packages/ketcher-core/src/application/editor/EditorHistory.ts b/packages/ketcher-core/src/application/editor/EditorHistory.ts index aaa223c684..6fdf0d200f 100644 --- a/packages/ketcher-core/src/application/editor/EditorHistory.ts +++ b/packages/ketcher-core/src/application/editor/EditorHistory.ts @@ -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'; @@ -35,7 +36,6 @@ export class EditorHistory { this.historyPointer = 0; EditorHistory._instance = this; - return this; } @@ -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--; @@ -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]; diff --git a/packages/ketcher-core/src/application/ketcher.ts b/packages/ketcher-core/src/application/ketcher.ts index 5a2f149e2b..56caf981bf 100644 --- a/packages/ketcher-core/src/application/ketcher.ts +++ b/packages/ketcher-core/src/application/ketcher.ts @@ -1,3 +1,4 @@ +import { Subscription } from 'subscription'; /**************************************************************************** * Copyright 2021 EPAM Systems * @@ -65,6 +66,7 @@ export class Ketcher { #editor: Editor; _indigo: Indigo; #eventBus: EventEmitter; + changeEvent: Subscription; get editor(): Editor { return this.#editor; @@ -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; diff --git a/packages/ketcher-react/src/script/editor/Editor.ts b/packages/ketcher-react/src/script/editor/Editor.ts index dfc58d16c3..171d4f535d 100644 --- a/packages/ketcher-react/src/script/editor/Editor.ts +++ b/packages/ketcher-react/src/script/editor/Editor.ts @@ -30,6 +30,7 @@ import { Scale, Struct, Vec2, + ketcherProvider, } from 'ketcher-core'; import { DOMSubscription, @@ -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, @@ -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'); } } @@ -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); } @@ -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'); } @@ -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'); } @@ -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(); @@ -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; } diff --git a/packages/ketcher-react/src/script/editor/utils/customOnChangeHandler.ts b/packages/ketcher-react/src/script/editor/utils/customOnChangeHandler.ts index 63814f237a..727b5d848d 100644 --- a/packages/ketcher-react/src/script/editor/utils/customOnChangeHandler.ts +++ b/packages/ketcher-react/src/script/editor/utils/customOnChangeHandler.ts @@ -40,158 +40,173 @@ type Data = { export function customOnChangeHandler(action, handler) { const data: Data[] = []; - - action.operations.reverse().forEach((operation) => { - const op = operation._inverted; - switch (op.type) { - case OperationType.ATOM_ADD: - case OperationType.ATOM_DELETE: - data.push({ - operation: op.type, - id: op.data.aid, - label: op.data.atom.label ? op.data.atom.label : '', - position: { - x: +op.data.pos.x.toFixed(2), - y: +op.data.pos.y.toFixed(2), - }, - }); - break; - - case OperationType.ATOM_ATTR: - data.push({ - operation: operation.type, - id: operation.data.aid, - attribute: operation.data.attribute, - from: operation.data.value, - to: operation.data2.value, - }); - break; - - case OperationType.ATOM_MOVE: - data.push({ - operation: op.type, - id: op.data.aid, - position: { - x: +op.data.d.x.toFixed(2), - y: +op.data.d.y.toFixed(2), - }, - }); - break; - - case OperationType.BOND_ADD: - case OperationType.BOND_DELETE: - data.push({ - operation: op.type, - id: op.data.bid, - }); - break; - - case OperationType.FRAGMENT_ADD: - case OperationType.FRAGMENT_DELETE: - data.push({ - operation: op.type, - id: op.frid, - }); - break; - - case OperationType.FRAGMENT_ADD_STEREO_ATOM: - case OperationType.FRAGMENT_DELETE_STEREO_ATOM: - data.push({ - operation: op.type, - atomId: op.data.aid, - fragId: op.data.frid, - }); - break; - - case OperationType.S_GROUP_ATOM_ADD: - case OperationType.S_GROUP_ATOM_REMOVE: - data.push({ - operation: op.type, - atomId: op.data.aid, - sGroupId: op.data.sgid, - }); - break; - - case OperationType.S_GROUP_CREATE: - case OperationType.S_GROUP_DELETE: - data.push({ - operation: op.type, - type: op.data.type, - sGroupId: op.data.sgid, - }); - break; - - case OperationType.RXN_ARROW_ADD: - case OperationType.RXN_ARROW_DELETE: - data.push({ - operation: op.type, - id: op.data.id, - mode: op.data.mode, - position: op.data.pos.map((pos) => ({ - x: +pos.x.toFixed(2), - y: +pos.y.toFixed(2), - })), - }); - break; - - case OperationType.RXN_ARROW_RESIZE: - data.push({ - operation: op.type, - id: op.data.id, - }); - break; - - case OperationType.RXN_ARROW_MOVE: - data.push({ - operation: op.type, - id: op.data.id, - position: { - x: +op.data.d.x.toFixed(2), - y: +op.data.d.y.toFixed(2), - }, - }); - break; - - case OperationType.R_GROUP_FRAGMENT: - data.push({ - operation: operation.type, - id: op.frid, - }); - break; - - case OperationType.SIMPLE_OBJECT_ADD: - case OperationType.SIMPLE_OBJECT_DELETE: - data.push({ - operation: op.type, - id: op.data.id, - mode: op.data.mode, - }); - break; - - case OperationType.SIMPLE_OBJECT_RESIZE: - data.push({ - operation: op.type, - id: op.data.id, - }); - break; - - case OperationType.SIMPLE_OBJECT_MOVE: - data.push({ - operation: operation.type, - id: operation.data.id, - position: { - x: +operation.data.d.x.toFixed(2), - y: +operation.data.d.y.toFixed(2), - }, - }); - break; - - default: - data.push({ - operation: op.type, - }); + if (action === undefined) { + return handler(); + } else { + if (window.isPolymerEditorTurnedOn) { + return handleMacroChanges(handler); + } else { + return handleMicroChanges(action, handler); } - }); - - return handler(data); + } + + function handleMacroChanges(handler) { + return handler(); + } + + function handleMicroChanges(action, handler) { + action.operations.reverse().forEach((operation) => { + const op = operation._inverted; + switch (op.type) { + case OperationType.ATOM_ADD: + case OperationType.ATOM_DELETE: + data.push({ + operation: op.type, + id: op.data.aid, + label: op.data.atom.label ? op.data.atom.label : '', + position: { + x: +op.data.pos.x.toFixed(2), + y: +op.data.pos.y.toFixed(2), + }, + }); + break; + + case OperationType.ATOM_ATTR: + data.push({ + operation: operation.type, + id: operation.data.aid, + attribute: operation.data.attribute, + from: operation.data.value, + to: operation.data2.value, + }); + break; + + case OperationType.ATOM_MOVE: + data.push({ + operation: op.type, + id: op.data.aid, + position: { + x: +op.data.d.x.toFixed(2), + y: +op.data.d.y.toFixed(2), + }, + }); + break; + + case OperationType.BOND_ADD: + case OperationType.BOND_DELETE: + data.push({ + operation: op.type, + id: op.data.bid, + }); + break; + + case OperationType.FRAGMENT_ADD: + case OperationType.FRAGMENT_DELETE: + data.push({ + operation: op.type, + id: op.frid, + }); + break; + + case OperationType.FRAGMENT_ADD_STEREO_ATOM: + case OperationType.FRAGMENT_DELETE_STEREO_ATOM: + data.push({ + operation: op.type, + atomId: op.data.aid, + fragId: op.data.frid, + }); + break; + + case OperationType.S_GROUP_ATOM_ADD: + case OperationType.S_GROUP_ATOM_REMOVE: + data.push({ + operation: op.type, + atomId: op.data.aid, + sGroupId: op.data.sgid, + }); + break; + + case OperationType.S_GROUP_CREATE: + case OperationType.S_GROUP_DELETE: + data.push({ + operation: op.type, + type: op.data.type, + sGroupId: op.data.sgid, + }); + break; + + case OperationType.RXN_ARROW_ADD: + case OperationType.RXN_ARROW_DELETE: + data.push({ + operation: op.type, + id: op.data.id, + mode: op.data.mode, + position: op.data.pos.map((pos) => ({ + x: +pos.x.toFixed(2), + y: +pos.y.toFixed(2), + })), + }); + break; + + case OperationType.RXN_ARROW_RESIZE: + data.push({ + operation: op.type, + id: op.data.id, + }); + break; + + case OperationType.RXN_ARROW_MOVE: + data.push({ + operation: op.type, + id: op.data.id, + position: { + x: +op.data.d.x.toFixed(2), + y: +op.data.d.y.toFixed(2), + }, + }); + break; + + case OperationType.R_GROUP_FRAGMENT: + data.push({ + operation: operation.type, + id: op.frid, + }); + break; + + case OperationType.SIMPLE_OBJECT_ADD: + case OperationType.SIMPLE_OBJECT_DELETE: + data.push({ + operation: op.type, + id: op.data.id, + mode: op.data.mode, + }); + break; + + case OperationType.SIMPLE_OBJECT_RESIZE: + data.push({ + operation: op.type, + id: op.data.id, + }); + break; + + case OperationType.SIMPLE_OBJECT_MOVE: + data.push({ + operation: operation.type, + id: operation.data.id, + position: { + x: +operation.data.d.x.toFixed(2), + y: +operation.data.d.y.toFixed(2), + }, + }); + break; + + default: + data.push({ + operation: op.type, + }); + } + }); + + return handler(data); + } }