Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: comment move and change events #7947

Merged
merged 4 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions core/comments/rendered_workspace_comment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,16 +101,15 @@ export class RenderedWorkspaceComment
}

/** Move the comment by the given amounts in workspace coordinates. */
moveBy(dx: number, dy: number, _reason?: string[] | undefined): void {
// TODO(#7909): Deal with reason when we add events.
moveBy(dx: number, dy: number, reason?: string[] | undefined): void {
const loc = this.getRelativeToSurfaceXY();
const newLoc = new Coordinate(loc.x + dx, loc.y + dy);
this.moveTo(newLoc);
this.moveTo(newLoc, reason);
}

/** Moves the comment to the given location in workspace coordinates. */
override moveTo(location: Coordinate): void {
super.moveTo(location);
override moveTo(location: Coordinate, reason?: string[] | undefined): void {
super.moveTo(location, reason);
this.view.moveTo(location);
}

Expand Down
22 changes: 21 additions & 1 deletion core/comments/workspace_comment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {Size} from '../utils/size.js';
import {Coordinate} from '../utils/coordinate.js';
import * as idGenerator from '../utils/idgenerator.js';
import * as eventUtils from '../events/utils.js';
import {CommentMove} from '../events/events_comment_move.js';

export class WorkspaceComment {
/** The unique identifier for this comment. */
Expand Down Expand Up @@ -72,9 +73,20 @@ export class WorkspaceComment {
}
}

/** Fires a comment change event. */
private fireChangeEvent(oldText: string, newText: string) {
if (eventUtils.isEnabled()) {
eventUtils.fire(
new (eventUtils.get(eventUtils.COMMENT_CHANGE))(this, oldText, newText),
);
}
}

/** Sets the text of the comment. */
setText(text: string) {
const oldText = this.text;
this.text = text;
this.fireChangeEvent(oldText, text);
}

/** Returns the text of the comment. */
Expand Down Expand Up @@ -166,8 +178,16 @@ export class WorkspaceComment {
}

/** Moves the comment to the given location in workspace coordinates. */
moveTo(location: Coordinate) {
moveTo(location: Coordinate, reason?: string[] | undefined) {
const event = new (eventUtils.get(eventUtils.COMMENT_MOVE))(
this,
) as CommentMove;
if (reason) event.setReason(reason);

this.location = location;

event.recordNew();
if (eventUtils.isEnabled()) eventUtils.fire(event);
}

/** Returns the position of the comment in workspace coordinates. */
Expand Down
9 changes: 6 additions & 3 deletions core/events/events_comment_change.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,16 @@ export class CommentChange extends CommentBase {
'the constructor, or call fromJson',
);
}
const comment = workspace.getCommentById(this.commentId);
// TODO: Remove the cast when we fix the type of getCommentById.
const comment = workspace.getCommentById(
this.commentId,
) as unknown as WorkspaceComment;
if (!comment) {
console.warn("Can't change non-existent comment: " + this.commentId);
return;
}
const contents = forward ? this.newContents_ : this.oldContents_;
if (!contents) {
if (contents === undefined) {
if (forward) {
throw new Error(
'The new contents is undefined. Either pass a value to ' +
Expand All @@ -142,7 +145,7 @@ export class CommentChange extends CommentBase {
'the constructor, or call fromJson',
);
}
comment.setContent(contents);
comment.setText(contents);
}
}

Expand Down
29 changes: 25 additions & 4 deletions core/events/events_comment_move.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,17 @@ export class CommentMove extends CommentBase {
/** The location of the comment after the move, in workspace coordinates. */
newCoordinate_?: Coordinate;

/**
* An explanation of what this move is for. Known values include:
* 'drag' -- A drag operation completed.
* 'snap' -- Comment got shifted to line up with the grid.
* 'inbounds' -- Block got pushed back into a non-scrolling workspace.
* 'create' -- Block created via deserialization.
* 'cleanup' -- Workspace aligned top-level blocks.
* Event merging may create multiple reasons: ['drag', 'inbounds', 'snap'].
*/
reason?: string[];

/**
* @param opt_comment The comment that is being moved. Undefined for a blank
* event.
Expand Down Expand Up @@ -70,6 +81,15 @@ export class CommentMove extends CommentBase {
this.newCoordinate_ = this.comment_.getRelativeToSurfaceXY();
}

/**
* Sets the reason for a move event.
*
* @param reason Why is this move happening? 'drag', 'bump', 'snap', ...
*/
setReason(reason: string[]) {
this.reason = reason;
}

/**
* Override the location before the move. Use this if you don't create the
* event until the end of the move, but you know the original location.
Expand Down Expand Up @@ -158,7 +178,10 @@ export class CommentMove extends CommentBase {
'the constructor, or call fromJson',
);
}
const comment = workspace.getCommentById(this.commentId);
// TODO: Remove cast when we update getCommentById.
const comment = workspace.getCommentById(
this.commentId,
) as unknown as WorkspaceComment;
if (!comment) {
console.warn("Can't move non-existent comment: " + this.commentId);
return;
Expand All @@ -172,9 +195,7 @@ export class CommentMove extends CommentBase {
'or call fromJson',
);
}
// TODO: Check if the comment is being dragged, and give up if so.
const current = comment.getRelativeToSurfaceXY();
comment.moveBy(target.x - current.x, target.y - current.y);
comment.moveTo(target);
}
}

Expand Down
40 changes: 40 additions & 0 deletions tests/mocha/workspace_comment_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,45 @@ suite('Workspace comment', function () {
this.workspace.id,
);
});

test('move events are fired when a comment is moved', function () {
this.renderedComment = new Blockly.comments.RenderedWorkspaceComment(
this.workspace,
);
const spy = createChangeListenerSpy(this.workspace);

this.renderedComment.moveTo(new Blockly.utils.Coordinate(42, 42));

assertEventFired(
spy,
Blockly.Events.CommentMove,
{
commentId: this.renderedComment.id,
oldCoordinate_: {x: 0, y: 0},
newCoordinate_: {x: 42, y: 42},
},
this.workspace.id,
);
});

test('change events are fired when a comments text is edited', function () {
this.renderedComment = new Blockly.comments.RenderedWorkspaceComment(
this.workspace,
);
const spy = createChangeListenerSpy(this.workspace);

this.renderedComment.setText('test text');

assertEventFired(
spy,
Blockly.Events.CommentChange,
{
commentId: this.renderedComment.id,
oldContents_: '',
newContents_: 'test text',
},
this.workspace.id,
);
});
});
});