feat: add firing of procedure events. (#6604)

* feat: add empty implementations of events

* chore: register all events

* chore: change assertions to shallow match properties

* feat: add firing events from the observable procedure map

* fix: make event not fired assertions actually fail

* chore: fixup typos in tests

* feat: add firing procedure model events

* feat: add firing parameter create and parameter delete events

* chore: reorganize event tests into suites

* feat: add firing parameter rename events

* chore: format

* chore: use tripple equals
This commit is contained in:
Beka Westberg
2022-11-10 16:38:32 -08:00
committed by GitHub
parent 92efdb91d1
commit 0532b5d1c0
21 changed files with 968 additions and 492 deletions

View File

@@ -28,6 +28,16 @@ import {CommentCreate, CommentCreateJson} from './events_comment_create.js';
import {CommentDelete} from './events_comment_delete.js';
import {CommentMove, CommentMoveJson} from './events_comment_move.js';
import {MarkerMove, MarkerMoveJson} from './events_marker_move.js';
import {ProcedureBase} from './events_procedure_base.js';
import {ProcedureChangeReturn} from './events_procedure_change_return.js';
import {ProcedureCreate} from './events_procedure_create.js';
import {ProcedureDelete} from './events_procedure_delete.js';
import {ProcedureEnable} from './events_procedure_enable.js';
import {ProcedureRename} from './events_procedure_rename.js';
import {ProcedureParameterBase} from './events_procedure_parameter_base.js';
import {ProcedureParameterCreate} from './events_procedure_parameter_create.js';
import {ProcedureParameterDelete} from './events_procedure_parameter_delete.js';
import {ProcedureParameterRename} from './events_procedure_parameter_rename.js';
import {Selected, SelectedJson} from './events_selected.js';
import {ThemeChange, ThemeChangeJson} from './events_theme_change.js';
import {ToolboxItemSelect, ToolboxItemSelectJson} from './events_toolbox_item_select.js';
@@ -77,6 +87,16 @@ export {FinishedLoading};
export {FinishedLoadingJson};
export {MarkerMove};
export {MarkerMoveJson};
export {ProcedureBase};
export {ProcedureChangeReturn};
export {ProcedureCreate};
export {ProcedureDelete};
export {ProcedureEnable};
export {ProcedureRename};
export {ProcedureParameterBase};
export {ProcedureParameterCreate};
export {ProcedureParameterDelete};
export {ProcedureParameterRename};
export {Selected};
export {SelectedJson};
export {ThemeChange};

View File

@@ -0,0 +1,19 @@
/**
* @license
* Copyright 2022 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import {Abstract as AbstractEvent} from './events_abstract.js';
import type {IProcedureModel} from '../interfaces/i_procedure_model.js';
import type {Workspace} from '../workspace.js';
export class ProcedureBase extends AbstractEvent {
isBlank = false;
constructor(workspace: Workspace, public readonly model: IProcedureModel) {
super();
this.workspaceId = workspace.id;
}
}

View File

@@ -0,0 +1,26 @@
/**
* @license
* Copyright 2022 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import {IProcedureModel} from '../interfaces/i_procedure_model.js';
import * as registry from '../registry.js';
import {Workspace} from '../workspace.js';
import {ProcedureBase} from './events_procedure_base.js';
import * as eventUtils from './utils.js';
export class ProcedureChangeReturn extends ProcedureBase {
constructor(
workpace: Workspace, model: IProcedureModel,
public readonly oldTypes: string[]|null) {
super(workpace, model);
}
}
registry.register(
registry.Type.EVENT, eventUtils.PROCEDURE_CHANGE_RETURN,
ProcedureChangeReturn);

View File

@@ -0,0 +1,16 @@
/**
* @license
* Copyright 2022 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import * as registry from '../registry.js';
import {ProcedureBase} from './events_procedure_base.js';
import * as eventUtils from './utils.js';
export class ProcedureCreate extends ProcedureBase {}
registry.register(
registry.Type.EVENT, eventUtils.PROCEDURE_CREATE, ProcedureCreate);

View File

@@ -0,0 +1,17 @@
/**
* @license
* Copyright 2022 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import * as registry from '../registry.js';
import {ProcedureBase} from './events_procedure_base.js';
import * as eventUtils from './utils.js';
export class ProcedureDelete extends ProcedureBase {}
registry.register(
registry.Type.EVENT, eventUtils.PROCEDURE_DELETE, ProcedureDelete);

View File

@@ -0,0 +1,17 @@
/**
* @license
* Copyright 2022 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import * as registry from '../registry.js';
import {ProcedureBase} from './events_procedure_base.js';
import * as eventUtils from './utils.js';
export class ProcedureEnable extends ProcedureBase {}
registry.register(
registry.Type.EVENT, eventUtils.PROCEDURE_ENABLE, ProcedureEnable);

View File

@@ -0,0 +1,11 @@
/**
* @license
* Copyright 2022 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import {ProcedureBase} from './events_procedure_base.js';
export class ProcedureParameterBase extends ProcedureBase {}

View File

@@ -0,0 +1,27 @@
/**
* @license
* Copyright 2022 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import {IParameterModel} from '../interfaces/i_parameter_model.js';
import {IProcedureModel} from '../interfaces/i_procedure_model.js';
import * as registry from '../registry.js';
import {Workspace} from '../workspace.js';
import {ProcedureParameterBase} from './events_procedure_parameter_base.js';
import * as eventUtils from './utils.js';
export class ProcedureParameterCreate extends ProcedureParameterBase {
constructor(
workspace: Workspace, procedure: IProcedureModel,
public readonly parameter: IParameterModel,
public readonly index: number) {
super(workspace, procedure);
}
}
registry.register(
registry.Type.EVENT, eventUtils.PROCEDURE_PARAMETER_CREATE,
ProcedureParameterCreate);

View File

@@ -0,0 +1,27 @@
/**
* @license
* Copyright 2022 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import {IParameterModel} from '../interfaces/i_parameter_model.js';
import {IProcedureModel} from '../interfaces/i_procedure_model.js';
import * as registry from '../registry.js';
import {Workspace} from '../workspace.js';
import {ProcedureParameterBase} from './events_procedure_parameter_base.js';
import * as eventUtils from './utils.js';
export class ProcedureParameterDelete extends ProcedureParameterBase {
constructor(
workspace: Workspace, procedure: IProcedureModel,
public readonly parameter: IParameterModel,
public readonly index: number) {
super(workspace, procedure);
}
}
registry.register(
registry.Type.EVENT, eventUtils.PROCEDURE_PARAMETER_DELETE,
ProcedureParameterDelete);

View File

@@ -0,0 +1,27 @@
/**
* @license
* Copyright 2022 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import {IParameterModel} from '../interfaces/i_parameter_model.js';
import {IProcedureModel} from '../interfaces/i_procedure_model.js';
import * as registry from '../registry.js';
import {Workspace} from '../workspace.js';
import {ProcedureParameterBase} from './events_procedure_parameter_base.js';
import * as eventUtils from './utils.js';
export class ProcedureParameterRename extends ProcedureParameterBase {
constructor(
workspace: Workspace, procedure: IProcedureModel,
public readonly parameter: IParameterModel,
public readonly oldName: string) {
super(workspace, procedure);
}
}
registry.register(
registry.Type.EVENT, eventUtils.PROCEDURE_PARAMETER_RENAME,
ProcedureParameterRename);

View File

@@ -0,0 +1,26 @@
/**
* @license
* Copyright 2022 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import {IProcedureModel} from '../interfaces/i_procedure_model.js';
import * as registry from '../registry.js';
import {Workspace} from '../workspace.js';
import {ProcedureBase} from './events_procedure_base.js';
import * as eventUtils from './utils.js';
export class ProcedureRename extends ProcedureBase {
type = eventUtils.PROCEDURE_RENAME;
constructor(
workspace: Workspace, model: IProcedureModel,
public readonly oldName: string) {
super(workspace, model);
}
}
registry.register(
registry.Type.EVENT, eventUtils.PROCEDURE_RENAME, ProcedureRename);

View File

@@ -240,6 +240,30 @@ export const COMMENT_MOVE = 'comment_move';
*/
export const FINISHED_LOADING = 'finished_loading';
/** Name of event that creates a procedure model. */
export const PROCEDURE_CREATE = 'procedure_create';
/** Name of event that deletes a procedure model. */
export const PROCEDURE_DELETE = 'procedure_delete';
/** Name of event that renames a procedure model. */
export const PROCEDURE_RENAME = 'procedure_rename';
/** Name of event that enables/disables a procedure model. */
export const PROCEDURE_ENABLE = 'procedure_enable';
/** Name of event that changes the returntype of a procedure model. */
export const PROCEDURE_CHANGE_RETURN = 'procedure_change_return';
/** Name of event that creates a procedure parameter. */
export const PROCEDURE_PARAMETER_CREATE = 'procedure_parameter_create';
/** Name of event that deletes a procedure parameter. */
export const PROCEDURE_PARAMETER_DELETE = 'procedure_parameter_delete';
/** Name of event that renames a procedure parameter. */
export const PROCEDURE_PARAMETER_RENAME = 'procedure_parameter_rename';
/**
* Type of events that cause objects to be bumped back into the visible
* portion of the workspace.

View File

@@ -0,0 +1,25 @@
/**
* @license
* Copyright 2022 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
/**
* An object that fires events optionally.
*
* @internal
*/
export interface IObservable {
startPublishing(): void;
stopPublishing(): void;
}
/**
* Type guard for checking if an object fulfills IObservable.
*
* @internal
*/
export function isObservable(obj: any): obj is IObservable {
return obj.startPublishing !== undefined && obj.stopPublishing !== undefined;
}

View File

@@ -4,11 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
/**
* The interface for the data model of a procedure parameter.
*
* @namespace Blockly.IParameterModel
*/
import {IProcedureModel} from './i_procedure_model';
/**
@@ -42,4 +38,7 @@ export interface IParameterModel {
* over time.
*/
getId(): string;
/** Sets the procedure model this parameter is associated with. */
setProcedureModel(model: IProcedureModel): this;
}

View File

@@ -4,8 +4,10 @@
* SPDX-License-Identifier: Apache-2.0
*/
import * as eventUtils from '../events/utils.js';
import {genUid} from '../utils/idgenerator.js';
import type {IParameterModel} from '../interfaces/i_parameter_model.js';
import type {IProcedureModel} from '../interfaces/i_procedure_model';
import {triggerProceduresUpdate} from './update_procedures.js';
import type {VariableModel} from '../variable_model.js';
import type {Workspace} from '../workspace.js';
@@ -14,6 +16,8 @@ import type {Workspace} from '../workspace.js';
export class ObservableParameterModel implements IParameterModel {
private id: string;
private variable: VariableModel;
private shouldFireEvents = false;
private procedureModel: IProcedureModel|null = null;
constructor(
private readonly workspace: Workspace, name: string, id?: string) {
@@ -26,11 +30,16 @@ export class ObservableParameterModel implements IParameterModel {
* Sets the name of this parameter to the given name.
*/
setName(name: string): this {
// TODO(#6516): Fire events.
if (name == this.variable.name) return this;
if (name === this.variable.name) return this;
const oldName = this.variable.name;
this.variable =
this.workspace.getVariable(name) ?? this.workspace.createVariable(name);
triggerProceduresUpdate(this.workspace);
if (this.shouldFireEvents) {
eventUtils.fire(
new (eventUtils.get(eventUtils.PROCEDURE_PARAMETER_RENAME))(
this.workspace, this.procedureModel, this, oldName));
}
return this;
}
@@ -76,4 +85,31 @@ export class ObservableParameterModel implements IParameterModel {
getVariableModel(): VariableModel {
return this.variable;
}
/**
* Tells the parameter model it should fire events.
*
* @internal
*/
startPublishing() {
this.shouldFireEvents = true;
}
/**
* Tells the parameter model it should not fire events.
*
* @internal
*/
stopPublishing() {
this.shouldFireEvents = false;
}
/** Sets the procedure model this parameter is a part of. */
setProcedureModel(model: IProcedureModel): this {
// TODO: Not sure if we want to do this, or accept it via the constructor.
// That means it could be non-null, but it would also break the fluent
// API.
this.procedureModel = model;
return this;
}
}

View File

@@ -4,10 +4,12 @@
* SPDX-License-Identifier: Apache-2.0
*/
import * as eventUtils from '../events/utils.js';
import {IProcedureMap} from '../interfaces/i_procedure_map.js';
import type {IProcedureModel} from '../interfaces/i_procedure_model.js';
import {isObservable} from '../interfaces/i_observable.js';
import {triggerProceduresUpdate} from './update_procedures.js';
import type {Workspace} from '../workspace.js';
import {IProcedureMap} from '../interfaces/i_procedure_map.js';
export class ObservableProcedureMap extends
@@ -20,8 +22,11 @@ export class ObservableProcedureMap extends
* Adds the given procedure model to the procedure map.
*/
override set(id: string, proc: IProcedureModel): this {
// TODO(#6516): Fire events.
if (this.get(id) === proc) return this;
super.set(id, proc);
eventUtils.fire(new (eventUtils.get(eventUtils.PROCEDURE_CREATE))(
this.workspace, proc));
if (isObservable(proc)) proc.startPublishing();
return this;
}
@@ -30,9 +35,13 @@ export class ObservableProcedureMap extends
* exists).
*/
override delete(id: string): boolean {
// TODO(#6516): Fire events.
const proc = this.get(id);
const existed = super.delete(id);
if (!existed) return existed;
triggerProceduresUpdate(this.workspace);
eventUtils.fire(new (eventUtils.get(eventUtils.PROCEDURE_DELETE))(
this.workspace, proc));
if (isObservable(proc)) proc.stopPublishing();
return existed;
}
@@ -40,8 +49,13 @@ export class ObservableProcedureMap extends
* Removes all ProcedureModels from the procedure map.
*/
override clear() {
// TODO(#6516): Fire events.
super.clear();
if (!this.size) return;
for (const id of this.keys()) {
const proc = this.get(id);
super.delete(id);
eventUtils.fire(new (eventUtils.get(eventUtils.PROCEDURE_DELETE))(
this.workspace, proc));
}
triggerProceduresUpdate(this.workspace);
}
@@ -50,7 +64,6 @@ export class ObservableProcedureMap extends
* blocks can find it.
*/
add(proc: IProcedureModel): this {
// TODO(#6516): Fire events.
// TODO(#6526): See if this method is actually useful.
return this.set(proc.getId(), proc);
}

View File

@@ -4,9 +4,11 @@
* SPDX-License-Identifier: Apache-2.0
*/
import * as eventUtils from '../events/utils.js';
import {genUid} from '../utils/idgenerator.js';
import type {IParameterModel} from '../interfaces/i_parameter_model.js';
import type {IProcedureModel} from '../interfaces/i_procedure_model.js';
import {isObservable} from '../interfaces/i_observable.js';
import {triggerProceduresUpdate} from './update_procedures.js';
import type {Workspace} from '../workspace.js';
@@ -17,6 +19,7 @@ export class ObservableProcedureModel implements IProcedureModel {
private parameters: IParameterModel[] = [];
private returnTypes: string[]|null = null;
private enabled = true;
private shouldFireEvents = false;
constructor(
private readonly workspace: Workspace, name: string, id?: string) {
@@ -26,9 +29,14 @@ export class ObservableProcedureModel implements IProcedureModel {
/** Sets the human-readable name of the procedure. */
setName(name: string): this {
// TODO(#6516): Fire events.
if (name === this.name) return this;
const prevName = this.name;
this.name = name;
triggerProceduresUpdate(this.workspace);
if (this.shouldFireEvents) {
eventUtils.fire(new (eventUtils.get(eventUtils.PROCEDURE_RENAME))(
this.workspace, this, prevName));
}
return this;
}
@@ -38,17 +46,46 @@ export class ObservableProcedureModel implements IProcedureModel {
* To move a parameter, first delete it, and then re-insert.
*/
insertParameter(parameterModel: IParameterModel, index: number): this {
// TODO(#6516): Fire events.
if (this.parameters[index] &&
this.parameters[index].getId() === parameterModel.getId()) {
return this;
}
this.parameters.splice(index, 0, parameterModel);
parameterModel.setProcedureModel(this);
if (isObservable(parameterModel)) {
if (this.shouldFireEvents) {
parameterModel.startPublishing();
} else {
parameterModel.stopPublishing();
}
}
triggerProceduresUpdate(this.workspace);
if (this.shouldFireEvents) {
eventUtils.fire(
new (eventUtils.get(eventUtils.PROCEDURE_PARAMETER_CREATE))(
this.workspace, this, parameterModel, index));
}
return this;
}
/** Removes the parameter at the given index from the parameter list. */
deleteParameter(index: number): this {
// TODO(#6516): Fire events.
if (!this.parameters[index]) return this;
const oldParam = this.parameters[index];
this.parameters.splice(index, 1);
triggerProceduresUpdate(this.workspace);
if (isObservable(oldParam)) {
oldParam.stopPublishing();
}
if (this.shouldFireEvents) {
eventUtils.fire(
new (eventUtils.get(eventUtils.PROCEDURE_PARAMETER_DELETE))(
this.workspace, this, oldParam, index));
}
return this;
}
@@ -67,9 +104,15 @@ export class ObservableProcedureModel implements IProcedureModel {
'The built-in ProcedureModel does not support typing. You need to ' +
'implement your own custom ProcedureModel.');
}
// Either they're both an empty array, or both null. Noop either way.
if (!!types === !!this.returnTypes) return this;
const oldReturnTypes = this.returnTypes;
this.returnTypes = types;
// TODO(#6516): Fire events.
triggerProceduresUpdate(this.workspace);
if (this.shouldFireEvents) {
eventUtils.fire(new (eventUtils.get(eventUtils.PROCEDURE_CHANGE_RETURN))(
this.workspace, this, oldReturnTypes));
}
return this;
}
@@ -78,9 +121,13 @@ export class ObservableProcedureModel implements IProcedureModel {
* all procedure caller blocks should be disabled as well.
*/
setEnabled(enabled: boolean): this {
// TODO(#6516): Fire events.
if (enabled === this.enabled) return this;
this.enabled = enabled;
triggerProceduresUpdate(this.workspace);
if (this.shouldFireEvents) {
eventUtils.fire(new (eventUtils.get(eventUtils.PROCEDURE_ENABLE))(
this.workspace, this));
}
return this;
}
@@ -120,4 +167,28 @@ export class ObservableProcedureModel implements IProcedureModel {
getEnabled(): boolean {
return this.enabled;
}
/**
* Tells the procedure model it should fire events.
*
* @internal
*/
startPublishing() {
this.shouldFireEvents = true;
for (const param of this.parameters) {
if (isObservable(param)) param.startPublishing();
}
}
/**
* Tells the procedure model it should not fire events.
*
* @internal
*/
stopPublishing() {
this.shouldFireEvents = false;
for (const param of this.parameters) {
if (isObservable(param)) param.stopPublishing();
}
}
}

View File

@@ -8,7 +8,7 @@ goog.declareModuleId('Blockly.test.event');
import * as Blockly from '../../build/src/core/blockly.js';
import {ASTNode} from '../../build/src/core/keyboard_nav/ast_node.js';
import {assertEventEquals, assertNthCallEventArgEquals, createFireChangeListenerSpy} from './test_helpers/events.js';
import {assertEventEquals, assertNthCallEventArgEquals, createChangeListenerSpy} from './test_helpers/events.js';
import {assertVariableValues} from './test_helpers/variables.js';
import {createGenUidStubWithReturns, sharedTestSetup, sharedTestTeardown, workspaceTeardown} from './test_helpers/setup_teardown.js';
import * as eventUtils from '../../build/src/core/events/utils.js';
@@ -962,7 +962,7 @@ suite('Events', function() {
suite('Firing', function() {
setup(function() {
this.changeListenerSpy = createFireChangeListenerSpy(this.workspace);
this.changeListenerSpy = createChangeListenerSpy(this.workspace);
});
test('Block dispose triggers Delete', function() {
@@ -984,7 +984,7 @@ suite('Events', function() {
this.clock.runAll();
this.eventsFireSpy.resetHistory();
const changeListenerSpy = createFireChangeListenerSpy(workspaceSvg);
const changeListenerSpy = createChangeListenerSpy(workspaceSvg);
block.dispose();
// Run all queued events.

File diff suppressed because it is too large Load Diff

View File

@@ -13,8 +13,10 @@ goog.declareModuleId('Blockly.test.helpers.events');
* calls on.
* @return {!SinonSpy} The created spy.
*/
export function createFireChangeListenerSpy(workspace) {
return sinon.spy(workspace, 'fireChangeListener');
export function createChangeListenerSpy(workspace) {
const spy = sinon.spy();
workspace.addChangeListener(spy);
return spy;
}
/**
@@ -140,7 +142,6 @@ export function assertEventFired(spy, instanceType, expectedProperties,
*/
export function assertEventNotFired(spy, instanceType, expectedProperties,
expectedWorkspaceId, expectedBlockId) {
expectedProperties.type = instanceType.prototype.type;
if (expectedWorkspaceId !== undefined) {
expectedProperties.workspaceId = expectedWorkspaceId;
}

View File

@@ -6,7 +6,7 @@
goog.declareModuleId('Blockly.test.workspaceSvg');
import {assertEventFired, assertEventNotFired, createFireChangeListenerSpy} from './test_helpers/events.js';
import {assertEventFired, assertEventNotFired, createChangeListenerSpy} from './test_helpers/events.js';
import {assertVariableValues} from './test_helpers/variables.js';
import {defineStackBlock} from './test_helpers/block_definitions.js';
import * as eventUtils from '../../build/src/core/events/utils.js';
@@ -163,7 +163,7 @@ suite('WorkspaceSvg', function() {
}
setup(function() {
defineStackBlock();
this.changeListenerSpy = createFireChangeListenerSpy(this.workspace);
this.changeListenerSpy = createChangeListenerSpy(this.workspace);
});
teardown(function() {
delete Blockly.Blocks['stack_block'];