From 5462b21b15883ea94bec8d8461416879bef8ee47 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Wed, 20 Mar 2024 23:25:41 +0000 Subject: [PATCH] fix: comment have XML save and load new workspace comments classes (#7931) * fix: have XML save and load new comment classes * chore: fix imports to resolve circular dependencies * chore: add round-trip tests * chore: skip failing test * fixup: PR comments --- core/comments/comment_view.ts | 4 +- core/xml.ts | 85 ++++++++++++----- tests/mocha/clipboard_test.js | 3 +- tests/mocha/serializer_test.js | 169 +++++++++++++++++++++++++++++++++ 4 files changed, 237 insertions(+), 24 deletions(-) diff --git a/core/comments/comment_view.ts b/core/comments/comment_view.ts index 05491f0ac..bc360a2c0 100644 --- a/core/comments/comment_view.ts +++ b/core/comments/comment_view.ts @@ -10,7 +10,9 @@ import * as dom from '../utils/dom.js'; import {Svg} from '../utils/svg.js'; import * as layers from '../layers.js'; import * as css from '../css.js'; -import {Coordinate, Size, browserEvents} from '../utils.js'; +import {Coordinate} from '../utils/coordinate.js'; +import {Size} from '../utils/size.js'; +import * as browserEvents from '../browser_events.js'; import * as touch from '../touch.js'; export class CommentView implements IRenderedElement { diff --git a/core/xml.ts b/core/xml.ts index 0f2c4f143..e8d4aa839 100644 --- a/core/xml.ts +++ b/core/xml.ts @@ -19,22 +19,21 @@ import * as utilsXml from './utils/xml.js'; import type {VariableModel} from './variable_model.js'; import * as Variables from './variables.js'; import type {Workspace} from './workspace.js'; -import {WorkspaceComment} from './workspace_comment.js'; -import {WorkspaceCommentSvg} from './workspace_comment_svg.js'; -import type {WorkspaceSvg} from './workspace_svg.js'; +import {WorkspaceSvg} from './workspace_svg.js'; import * as renderManagement from './render_management.js'; +import {WorkspaceComment} from './comments/workspace_comment.js'; +import {RenderedWorkspaceComment} from './comments/rendered_workspace_comment.js'; +import {Coordinate} from './utils/coordinate.js'; /** * Encode a block tree as XML. * * @param workspace The workspace containing blocks. - * @param opt_noId True if the encoder should skip the block IDs. + * @param skipId True if the encoder should skip the block IDs. False by + * default. * @returns XML DOM element. */ -export function workspaceToDom( - workspace: Workspace, - opt_noId?: boolean, -): Element { +export function workspaceToDom(workspace: Workspace, skipId = false): Element { const treeXml = utilsXml.createElement('xml'); const variablesElement = variablesToDom( Variables.allUsedVarModels(workspace), @@ -42,19 +41,41 @@ export function workspaceToDom( if (variablesElement.hasChildNodes()) { treeXml.appendChild(variablesElement); } - const comments = workspace.getTopComments(true); - for (let i = 0; i < comments.length; i++) { - const comment = comments[i]; - treeXml.appendChild(comment.toXmlWithXY(opt_noId)); + for (const comment of workspace.getTopComments()) { + treeXml.appendChild( + saveWorkspaceComment(comment as AnyDuringMigration, skipId), + ); } const blocks = workspace.getTopBlocks(true); for (let i = 0; i < blocks.length; i++) { const block = blocks[i]; - treeXml.appendChild(blockToDomWithXY(block, opt_noId)); + treeXml.appendChild(blockToDomWithXY(block, skipId)); } return treeXml; } +/** Serializes the given workspace comment to XML. */ +function saveWorkspaceComment( + comment: WorkspaceComment, + skipId = false, +): Element { + const elem = utilsXml.createElement('comment'); + if (!skipId) elem.setAttribute('id', comment.id); + + elem.setAttribute('x', `${comment.getRelativeToSurfaceXY().x}`); + elem.setAttribute('y', `${comment.getRelativeToSurfaceXY().y}`); + elem.setAttribute('w', `${comment.getSize().width}`); + elem.setAttribute('h', `${comment.getSize().height}`); + + if (comment.getText()) elem.textContent = comment.getText(); + if (comment.isCollapsed()) elem.setAttribute('collapsed', 'true'); + if (!comment.isOwnEditable()) elem.setAttribute('editable', 'false'); + if (!comment.isOwnMovable()) elem.setAttribute('movable', 'false'); + if (!comment.isOwnDeletable()) elem.setAttribute('deletable', 'false'); + + return elem; +} + /** * Encode a list of variables as XML. * @@ -443,15 +464,7 @@ export function domToWorkspace(xml: Element, workspace: Workspace): string[] { } else if (name === 'shadow') { throw TypeError('Shadow block cannot be a top-level block.'); } else if (name === 'comment') { - if (workspace.rendered) { - WorkspaceCommentSvg.fromXmlRendered( - xmlChildElement, - workspace as WorkspaceSvg, - width, - ); - } else { - WorkspaceComment.fromXml(xmlChildElement, workspace); - } + loadWorkspaceComment(xmlChildElement, workspace); } else if (name === 'variables') { if (variablesFirst) { domToVariables(xmlChildElement, workspace); @@ -478,6 +491,34 @@ export function domToWorkspace(xml: Element, workspace: Workspace): string[] { return newBlockIds; } +/** Deserializes the given comment state into the given workspace. */ +function loadWorkspaceComment( + elem: Element, + workspace: Workspace, +): WorkspaceComment { + const id = elem.getAttribute('id') ?? undefined; + const comment = workspace.rendered + ? new RenderedWorkspaceComment(workspace as WorkspaceSvg, id) + : new WorkspaceComment(workspace, id); + + comment.setText(elem.textContent ?? ''); + + const x = parseInt(elem.getAttribute('x') ?? '', 10); + const y = parseInt(elem.getAttribute('y') ?? '', 10); + if (!isNaN(x) && !isNaN(y)) comment.moveTo(new Coordinate(x, y)); + + const w = parseInt(elem.getAttribute('w') ?? '', 10); + const h = parseInt(elem.getAttribute('h') ?? '', 10); + if (!isNaN(w) && !isNaN(h)) comment.setSize(new Size(w, h)); + + if (elem.getAttribute('collapsed') === 'true') comment.setCollapsed(true); + if (elem.getAttribute('editable') === 'false') comment.setEditable(false); + if (elem.getAttribute('movable') === 'false') comment.setMovable(false); + if (elem.getAttribute('deletable') === 'false') comment.setDeletable(false); + + return comment; +} + /** * Decode an XML DOM and create blocks on the workspace. Position the new * blocks immediately below prior blocks, aligned by their starting edge. diff --git a/tests/mocha/clipboard_test.js b/tests/mocha/clipboard_test.js index f134b1d77..fb0c41882 100644 --- a/tests/mocha/clipboard_test.js +++ b/tests/mocha/clipboard_test.js @@ -114,7 +114,8 @@ suite('Clipboard', function () { }); suite('pasting comments', function () { - test('pasted comments are bumped to not overlap', function () { + // TODO: Reenable test when we readd copy-paste. + test.skip('pasted comments are bumped to not overlap', function () { Blockly.Xml.domToWorkspace( Blockly.utils.xml.textToDom( '', diff --git a/tests/mocha/serializer_test.js b/tests/mocha/serializer_test.js index 500c10684..290f7fa45 100644 --- a/tests/mocha/serializer_test.js +++ b/tests/mocha/serializer_test.js @@ -1868,12 +1868,181 @@ Serializer.Mutations.testSuites = [ Serializer.Mutations.Procedure, ]; +Serializer.Comments = new SerializerTestSuite('Comments'); + +Serializer.Comments.Coordinates = new SerializerTestSuite('Coordinates'); +Serializer.Comments.Coordinates.Basic = new SerializerTestCase( + 'Basic', + '' + + '' + + '' + + '', +); +Serializer.Comments.Coordinates.Negative = new SerializerTestCase( + 'Negative', + '' + + '' + + '' + + '', +); +Serializer.Comments.Coordinates.Zero = new SerializerTestCase( + 'Zero', + '' + + '' + + '' + + '', +); +Serializer.Comments.Coordinates.testCases = [ + Serializer.Comments.Coordinates.Basic, + Serializer.Comments.Coordinates.Negative, + Serializer.Comments.Coordinates.Zero, +]; + +Serializer.Comments.Size = new SerializerTestSuite('Size'); +Serializer.Comments.Size.Basic = new SerializerTestCase( + 'Basic', + '' + + '' + + '' + + '', +); +Serializer.Comments.Size.testCases = [Serializer.Comments.Size.Basic]; + +Serializer.Comments.Text = new SerializerTestSuite('Text'); +Serializer.Comments.Text.Symbols = new SerializerTestCase( + 'Symbols', + '' + + '' + + '~`!@#$%^*()_+-={[}]|\\:;,.?/' + + '' + + '', +); +Serializer.Comments.Text.EscapedSymbols = new SerializerTestCase( + 'EscapedSymbols', + '' + + '' + + '&<>' + + '' + + '', +); +Serializer.Comments.Text.SingleQuotes = new SerializerTestCase( + 'SingleQuotes', + '' + + '' + + "'test'" + + '' + + '', +); +Serializer.Comments.Text.DoubleQuotes = new SerializerTestCase( + 'DoubleQuotes', + '' + + '' + + '"test"' + + '' + + '', +); +Serializer.Comments.Text.Numbers = new SerializerTestCase( + 'Numbers', + '' + + '' + + '1234567890a123a123a' + + '' + + '', +); +Serializer.Comments.Text.Emoji = new SerializerTestCase( + 'Emoji', + '' + + '' + + '😀👋🏿👋🏾👋🏽👋🏼👋🏻😀❤❤❤' + + '' + + '', +); +Serializer.Comments.Text.Russian = new SerializerTestCase( + 'Russian', + '' + + '' + + 'ты любопытный кот' + + '' + + '', +); +Serializer.Comments.Text.Japanese = new SerializerTestCase( + 'Japanese', + '' + + '' + + 'あなたは好奇心旺盛な猫です' + + '' + + '', +); +Serializer.Comments.Text.Zalgo = new SerializerTestCase( + 'Zalgo', + '' + + '' + + 'z̴̪͈̲̜͕̽̈̀͒͂̓̋̉̍a̸̧̧̜̻̘̤̫̱̲͎̞̻͆̋ļ̸̛̖̜̳͚̖͔̟̈́͂̉̀͑̑͑̎ǵ̸̫̳̽̐̃̑̚̕o̶͇̫͔̮̼̭͕̹̘̬͋̀͆̂̇̋͊̒̽' + + '' + + '', +); +Serializer.Comments.Text.testCases = [ + Serializer.Comments.Text.Symbols, + Serializer.Comments.Text.EscapedSymbols, + Serializer.Comments.Text.SingleQuotes, + Serializer.Comments.Text.DoubleQuotes, + Serializer.Comments.Text.Numbers, + Serializer.Comments.Text.Emoji, + Serializer.Comments.Text.Russian, + Serializer.Comments.Text.Japanese, + Serializer.Comments.Text.Zalgo, +]; + +Serializer.Comments.Attributes = new SerializerTestSuite('Attributes'); +Serializer.Comments.Attributes.Collapsed = new SerializerTestCase( + 'Collapsed', + '' + + '' + + '' + + '', +); +Serializer.Comments.Attributes.NotEditable = new SerializerTestCase( + 'NotEditable', + '' + + '' + + '' + + '', +); +Serializer.Comments.Attributes.NotMovable = new SerializerTestCase( + 'NotMovable', + '' + + '' + + '' + + '', +); +Serializer.Comments.Attributes.NotDeletable = new SerializerTestCase( + 'NotDeletable', + '' + + '' + + '' + + '', +); +Serializer.Comments.Attributes.testCases = [ + Serializer.Comments.Attributes.Collapsed, + Serializer.Comments.Attributes.NotEditable, + Serializer.Comments.Attributes.NotMovable, + Serializer.Comments.Attributes.NotDeletable, +]; + +Serializer.Comments.testSuites = [ + Serializer.Comments.Coordinates, + Serializer.Comments.Size, + Serializer.Comments.Text, + Serializer.Comments.Attributes, +]; + Serializer.testSuites = [ Serializer.Attributes, Serializer.Fields, Serializer.Icons, Serializer.Connections, Serializer.Mutations, + Serializer.Comments, ]; const runSerializerTestSuite = (serializer, deserializer, testSuite) => {