diff --git a/packages/client/src/base/command-stack.ts b/packages/client/src/base/command-stack.ts index ddb0ef34..a740fecd 100644 --- a/packages/client/src/base/command-stack.ts +++ b/packages/client/src/base/command-stack.ts @@ -14,6 +14,7 @@ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 ********************************************************************************/ import { + CommandExecutionContext, CommandStack, Disposable, DisposableCollection, @@ -51,6 +52,10 @@ export class GLSPCommandStack extends CommandStack implements Disposable { return this.editorContext.onModelRootChanged; } + /** + * Client-side undo/redo is not supported in GLSP. The server is responsible for handling undo/redo requests. + * If this method get called it's probably a mistake and a warning is logged + */ override undo(): Promise { this.logger.warn( this, @@ -59,6 +64,10 @@ export class GLSPCommandStack extends CommandStack implements Disposable { return this.currentModel; } + /** + * Client-side undo/redo is not supported in GLSP. The server is responsible for handling undo/redo requests. + * If this method get called it's probably a mistake and a warning is logged + */ override redo(): Promise { this.logger.warn( this, @@ -67,14 +76,29 @@ export class GLSPCommandStack extends CommandStack implements Disposable { return this.currentModel; } + /** + * Client-side undo/redo is not supported in GLSP. + * To avoid unnecessary infraction with the command stack (pushing/merging/popping commands) + * related methods are overridden to no-ops. + */ + protected override pushToUndoStack(command: ICommand): void { + // no-op + } + + /** + * Client-side undo/redo is not supported in GLSP. + * To avoid unnecessary infraction with the command stack (pushing/merging/popping commands) + * related methods are overridden to no-ops. + */ + protected override mergeOrPush(command: ICommand, context: CommandExecutionContext): void { + // no-op + } override async execute(command: ICommand): Promise { + const result = await super.execute(command); if (command instanceof SetModelCommand || command instanceof UpdateModelCommand) { - const result = await super.execute(command); this.notifyListeners(result); - return result; } - - return super.execute(command); + return result; } protected notifyListeners(root: Readonly): void { diff --git a/packages/client/src/base/feedback/feedback-action-dispatcher-default.ts b/packages/client/src/base/feedback/feedback-action-dispatcher-default.ts index 2bc703cb..f81a36f1 100644 --- a/packages/client/src/base/feedback/feedback-action-dispatcher-default.ts +++ b/packages/client/src/base/feedback/feedback-action-dispatcher-default.ts @@ -27,20 +27,23 @@ import { TYPES, toTypeGuard } from '@eclipse-glsp/sprotty'; -import { inject, injectable } from 'inversify'; +import { inject, injectable, preDestroy } from 'inversify'; +import { IFeedbackActionDispatcher, IFeedbackEmitter, MaybeActions } from './feedback-action-dispatcher'; import { getFeedbackRank } from './feedback-command'; import { FeedbackEmitter } from './feedback-emitter'; -import { IFeedbackActionDispatcher, IFeedbackEmitter, MaybeActions } from './feedback-action-dispatcher'; @injectable() -export class FeedbackActionDispatcher implements IFeedbackActionDispatcher { +export class FeedbackActionDispatcher implements IFeedbackActionDispatcher, Disposable { protected registeredFeedback: Map = new Map(); - @inject(TYPES.IActionDispatcherProvider) protected actionDispatcher: () => Promise; + @inject(TYPES.IActionDispatcher) protected actionDispatcher: IActionDispatcher; + @inject(TYPES.ILogger) protected logger: ILogger; @inject(ActionHandlerRegistry) protected actionHandlerRegistry: ActionHandlerRegistry; + protected isDisposed = false; + registerFeedback(feedbackEmitter: IFeedbackEmitter, feedbackActions: Action[], cleanupActions?: MaybeActions): Disposable { if (feedbackEmitter instanceof GModelElement) { this.logger.log( @@ -109,11 +112,19 @@ export class FeedbackActionDispatcher implements IFeedbackActionDispatcher { protected async dispatchFeedback(actions: Action[], feedbackEmitter: IFeedbackEmitter): Promise { try { - const actionDispatcher = await this.actionDispatcher(); - await actionDispatcher.dispatchAll(actions); + if (this.isDisposed) { + return; + } + await this.actionDispatcher.dispatchAll(actions); this.logger.info(this, `Dispatched feedback actions for ${feedbackEmitter}`); } catch (reason) { this.logger.error(this, 'Failed to dispatch feedback actions', reason); } } + + @preDestroy() + dispose(): void { + this.registeredFeedback.clear(); + this.isDisposed = true; + } } diff --git a/packages/client/src/base/model/diagram-loader.ts b/packages/client/src/base/model/diagram-loader.ts index 2725cf8e..a5b4b88c 100644 --- a/packages/client/src/base/model/diagram-loader.ts +++ b/packages/client/src/base/model/diagram-loader.ts @@ -18,14 +18,13 @@ import { AnyObject, ApplicationIdProvider, Args, - EMPTY_ROOT, GLSPClient, IActionDispatcher, InitializeParameters, LazyInjector, MaybePromise, + RequestAction, RequestModelAction, - SetModelAction, StatusAction, TYPES, hasNumberProp @@ -44,19 +43,23 @@ export interface IDiagramOptions { * corresponding client session. */ clientId: string; + /** * The diagram type i.e. diagram language this diagram is associated with. */ diagramType: string; + /** * The provider function to retrieve the GLSP client used by this diagram to communicate with the server. - * Multiple invocations of the provder function should always return the same `GLSPClient` instance. + * Multiple invocations of the provider function should always return the same {@link GLSPClient} instance. */ glspClientProvider: () => Promise; + /** * The file source URI associated with this diagram. */ sourceUri?: string; + /** * The initial edit mode of diagram. Defaults to `editable`. */ @@ -74,24 +77,28 @@ export interface IDiagramStartup extends Partial { * first hook that is invoked directly after {@link DiagramLoader.load} is called. */ preLoadDiagram?(): MaybePromise; + /** - * Hook for services that want to execute code before the underlying GLSP client is configured and the server is initialized. + * Hook for services that want to execute code before the underlying {@link GLSPClient} is configured and the server is initialized. */ preInitialize?(): MaybePromise; + /** - * Hook for services that want to execute code before the initial model loading request (i.e. `RequestModelAction`) but + * Hook for services that want to execute code before the initial model loading request (i.e. {@link RequestModelAction}) but * after the underlying GLSP client has been configured and the server is initialized. */ preRequestModel?(): MaybePromise; + /** - * Hook for services that want to execute code after the initial model loading request (i.e. `RequestModelAction`). - * Note that this hook is invoked directly after the `RequestModelAction` has been dispatched. It does not necessarily wait + * Hook for services that want to execute code after the initial model loading request (i.e. {@link RequestModelAction}). + * Note that this hook is invoked directly after the {@link RequestModelAction} has been dispatched. It does not necessarily wait * until the client-server update roundtrip is completed. If you need to wait until the diagram is fully initialized use the * {@link postModelInitialization} hook. */ + postRequestModel?(): MaybePromise; - /* Hook for services that want to execute code after the diagram model is fully initialized - * (i.e. `ModelInitializationConstraint` is completed). + /** Hook for services that want to execute code after the diagram model is fully initialized + * (i.e. {@link ModelInitializationConstraint} is completed). */ postModelInitialization?(): MaybePromise; } @@ -119,7 +126,7 @@ export interface DiagramLoadingOptions { requestModelOptions?: Args; /** - * Optional partial {@link InitializeParameters} that should be used for `initializeServer` request if the underlying + * Optional partial {@link InitializeParameters} that should be used for {@link GLSPClient.initializeServer} request if the underlying * {@link GLSPClient} has not been initialized yet. */ initializeParameters?: Partial; @@ -180,14 +187,15 @@ export class DiagramLoader { }, enableNotifications: options.enableNotifications ?? true }; - // Set empty place holder model until actual model from server is available - await this.actionDispatcher.dispatch(SetModelAction.create(EMPTY_ROOT)); + // Ensure that the action dispatcher is initialized before starting the diagram loading process + await this.actionDispatcher.initialize?.(); await this.invokeStartupHook('preInitialize'); await this.initialize(resolvedOptions); await this.invokeStartupHook('preRequestModel'); await this.requestModel(resolvedOptions); await this.invokeStartupHook('postRequestModel'); - this.modelInitializationConstraint.onInitialized(() => this.invokeStartupHook('postModelInitialization')); + await this.modelInitializationConstraint.onInitialized(); + await this.invokeStartupHook('postModelInitialization'); } protected async invokeStartupHook(hook: keyof Omit): Promise { @@ -201,10 +209,9 @@ export class DiagramLoader { } protected async requestModel(options: ResolvedDiagramLoadingOptions): Promise { - const response = await this.actionDispatcher.request( - RequestModelAction.create({ options: options.requestModelOptions }) + await this.actionDispatcher.dispatch( + RequestModelAction.create({ options: options.requestModelOptions, requestId: RequestAction.generateRequestId() }) ); - return this.actionDispatcher.dispatch(response); } protected async initialize(options: ResolvedDiagramLoadingOptions): Promise { diff --git a/packages/client/src/features/accessibility/toast/toast-tool.ts b/packages/client/src/features/accessibility/toast/toast-tool.ts index 25d2be21..5a139790 100644 --- a/packages/client/src/features/accessibility/toast/toast-tool.ts +++ b/packages/client/src/features/accessibility/toast/toast-tool.ts @@ -14,7 +14,7 @@ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 ********************************************************************************/ -import { Action, IActionDispatcher, IActionHandler, ICommand, TYPES } from '@eclipse-glsp/sprotty'; +import { Action, IActionDispatcher, IActionHandler, ICommand, SetUIExtensionVisibilityAction, TYPES } from '@eclipse-glsp/sprotty'; import { inject, injectable } from 'inversify'; import { EditorContextService } from '../../../base/editor-context-service'; import { IDiagramStartup } from '../../../base/model/diagram-loader'; @@ -99,7 +99,7 @@ export class Toast extends GLSPAbstractUIExtension implements IActionHandler, ID } preInitialize(): void { - this.show(this.editorContext.modelRoot); + this.actionDispatcher.dispatch(SetUIExtensionVisibilityAction.create({ extensionId: Toast.ID, visible: true })); } values(obj: { [key: symbol]: ToastOptions }): ToastOptions[] { diff --git a/packages/client/src/features/status/status-overlay.ts b/packages/client/src/features/status/status-overlay.ts index e49d91d1..c226e400 100644 --- a/packages/client/src/features/status/status-overlay.ts +++ b/packages/client/src/features/status/status-overlay.ts @@ -13,9 +13,15 @@ * * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 ********************************************************************************/ -import { IActionDispatcher, IActionHandler, StatusAction, TYPES, codiconCSSClasses } from '@eclipse-glsp/sprotty'; +import { + IActionDispatcher, + IActionHandler, + SetUIExtensionVisibilityAction, + StatusAction, + TYPES, + codiconCSSClasses +} from '@eclipse-glsp/sprotty'; import { inject, injectable } from 'inversify'; -import { EditorContextService } from '../../base/editor-context-service'; import { IDiagramStartup } from '../../base/model/diagram-loader'; import { GLSPAbstractUIExtension } from '../../base/ui-extension/ui-extension'; @@ -29,9 +35,6 @@ export class StatusOverlay extends GLSPAbstractUIExtension implements IActionHan @inject(TYPES.IActionDispatcher) protected actionDispatcher: IActionDispatcher; - @inject(EditorContextService) - protected editorContext: EditorContextService; - protected statusIconDiv?: HTMLDivElement; protected statusMessageDiv?: HTMLDivElement; protected pendingTimeout?: number; @@ -115,7 +118,7 @@ export class StatusOverlay extends GLSPAbstractUIExtension implements IActionHan } } - preInitialize(): void { - this.show(this.editorContext.modelRoot); + preInitialize(): Promise { + return this.actionDispatcher.dispatch(SetUIExtensionVisibilityAction.create({ extensionId: this.id(), visible: true })); } } diff --git a/packages/client/src/features/tool-palette/tool-palette.ts b/packages/client/src/features/tool-palette/tool-palette.ts index d9200a7e..e74f75e8 100644 --- a/packages/client/src/features/tool-palette/tool-palette.ts +++ b/packages/client/src/features/tool-palette/tool-palette.ts @@ -474,17 +474,21 @@ export class ToolPalette extends GLSPAbstractUIExtension implements IActionHandl this.createBody(); } - async preRequestModel(): Promise { + /** + * @deprecated This hook is no longer used by the ToolPalette. + * It is kept for compatibility reasons and will be removed in the future. + * Move initialization logic to the `postRequestModel` method. + * This ensures that tool palette initialization does not block the diagram loading process. + */ + async preRequestModel(): Promise {} + + async postRequestModel(): Promise { await this.setPaletteItems(); if (!this.editorContext.isReadonly) { this.show(this.editorContext.modelRoot); } } - async postRequestModel(): Promise { - this.reloadPaletteBody(); - } - protected async setPaletteItems(): Promise { const requestAction = RequestContextActions.create({ contextId: ToolPalette.ID, @@ -506,7 +510,7 @@ export class ToolPalette extends GLSPAbstractUIExtension implements IActionHandl } protected async reloadPaletteBody(): Promise { - if (this.dynamic) { + if (!this.editorContext.isReadonly && this.dynamic) { await this.setPaletteItems(); this.paletteItemsCopy = []; this.requestFilterUpdate(this.searchField.value); diff --git a/packages/glsp-sprotty/src/api-override.ts b/packages/glsp-sprotty/src/api-override.ts index 81dc464a..06b75fda 100644 --- a/packages/glsp-sprotty/src/api-override.ts +++ b/packages/glsp-sprotty/src/api-override.ts @@ -135,6 +135,13 @@ export interface IVNodePostprocessor extends SIVNodePostprocessor { } export interface IActionDispatcher extends SIActionDispatcher { + /** + * Optional method to initialize the action dispatcher. + * Implementation can use this as a hook to perform any initialization tasks, + * like registering action handlers or setting up the initial diagram. + * Called by the `DiagramLoader` when starting the loading process. + */ + initialize?(): Promise; /** * Dispatch an action by querying all handlers that are registered for its kind. * The returned promise is resolved when all handler results (commands or actions)