Move clipboard functions to a separate namespace #5237

rachel-fenichel · 2021-07-28T19:02:51Z

Moves clipboard functions copy, paste, and duplicate out of blockly.js.
Moves clipboard state clipboardXml_, clipboardTypeCounts_, and clipboardSource_ out of blockly.js
Aliases old functions and properties to refer to new ones.
Updates tests that spy on specific functions.
Updates callers inside core.

This is part of #5208

There's an API question here: should clipboard xml, type counts, and source be exported? The keyboard nav plugin accesses them directly, but with a note that it needs a better way to do that.

I propose that there should instead be a getter for clipboardInfo that returns an object containing all three of those properties, but as copies (and effectively read-only).

I also propose that we remove clipboardXml_, clipboardTypeCounts_, and clipboardSource_ from blockly.js entirely and not export those three from clipboard.

alschmiedt

I agree with everything proposed in the PR description. We just also need to make an issue to update keyboard nav to use the new API before releasing.

alschmiedt · 2021-07-29T00:16:49Z

core/clipboard.js

+    typeCounts = data.typeCounts;
+  }
+};
+exports.copy = copy;


I think @Package is suppose to go above this line now.

rachel-fenichel · 2021-07-29T19:56:59Z

Tracking -samples update in RaspberryPiFoundation/blockly-samples#833

This PR also closes #4600

rachel-fenichel · 2021-07-29T19:57:38Z

core/blockly.js

-        Blockly.Blocks[typename] = {
-          init: Blockly.jsonInitFactory_(elem)
-        };
+        Blockly.Blocks[typename] = {init: Blockly.jsonInitFactory_(elem)};


Clang-format did this.

rachel-fenichel · 2021-07-29T19:58:03Z

core/blockly.js

    Blockly.hideChaff();
    if (selected.outputConnection) {
-      // Do not attempt to heal rows (https://github.com/google/blockly/issues/4832)
+      // Do not attempt to heal rows


clang-format did this.

rachel-fenichel · 2021-07-29T19:58:10Z

core/blockly.js

-      'Blockly.svgSize',
-      'March 2021',
-      'March 2022',
+      'Blockly.svgSize', 'March 2021', 'March 2022',


clang-format did this.

alschmiedt · 2021-07-29T20:19:16Z

core/blockly.js

- * @type {Blockly.WorkspaceSvg}
- * @private
+ * Get the current contents of the clipboard and associated metadata.
+ * @return {{xml: ?Element, source: ?Blockly.WorkspaceSvg, typeCounts: ?Object}}


If we could provide a better type for typeCounts now that this is public I think that would be really helpful.

I switched all of this to ICopyable.CopyData, which is an existing type.

alschmiedt · 2021-07-29T20:19:29Z

core/blockly.js

+ * Get the current contents of the clipboard and associated metadata.
+ * @return {{xml: ?Element, source: ?Blockly.WorkspaceSvg, typeCounts: ?Object}}
+ *     An object containing the clipboard contents and associated metadata.
+ * @public


I think we aren't adding @public.

Deleted function.

alschmiedt · 2021-07-29T20:22:39Z

core/blockly.js

- * @private
- */
-Blockly.clipboardTypeCounts_ = null;
+Blockly.getClipboardInfo = Blockly.clipboard.getClipboardInfo;


Do we need to add this here since it is a new API?

I removed this for now. However, we will have to revisit when we remove declareLegacyNamespace in clipboard.js. Since the clipboard didn't exist at all before this quarter, and I prefer to keep it internal, I may end up adding this single function at the top level.

BeksOmega · 2021-07-29T20:28:13Z

Just tagging this as something I was planning on switching over to the JSO system. But since this is such a big change it may be better to wait.

rachel-fenichel · 2021-07-30T00:22:16Z

@alschmiedt I made some significant changes, so please take another look.

Changes:

Switched to storing data as an ICopyable.CopyData instead of storing each component individually.
Got rid of the exported function getClipboardInfo and also removed it from blockly.js--it's not clear that it's the right API, and we have an issue tracking that.

Move clipboard functions to a separate namespace

9612546

rachel-fenichel requested a review from a team as a code owner July 28, 2021 19:02

rachel-fenichel requested review from alschmiedt and maribethb and removed request for maribethb July 28, 2021 19:02

github-actions bot added this to the 2021_q3_release milestone Jul 28, 2021

github-actions bot added the type: cleanup label Jul 28, 2021

rachel-fenichel assigned alschmiedt Jul 28, 2021

alschmiedt reviewed Jul 29, 2021

View reviewed changes

rachel-fenichel added the breaking change Used to mark a PR or issue that changes our public APIs. label Jul 29, 2021

Switch to getter for keyboard contents.

d58a878

github-actions bot removed the breaking change Used to mark a PR or issue that changes our public APIs. label Jul 29, 2021

Fix annotations

714bb61

rachel-fenichel mentioned this pull request Jul 29, 2021

Keyboard nav uses non-public properties in paste RaspberryPiFoundation/blockly-samples#833

Closed

rachel-fenichel commented Jul 29, 2021

View reviewed changes

rachel-fenichel added the breaking change Used to mark a PR or issue that changes our public APIs. label Jul 29, 2021

alschmiedt approved these changes Jul 29, 2021

View reviewed changes

Use CopyData type

f7549f7

github-actions bot removed the breaking change Used to mark a PR or issue that changes our public APIs. label Jul 30, 2021

Remove getClipboardInfo

f72c5ce

alschmiedt approved these changes Jul 30, 2021

View reviewed changes

rachel-fenichel merged commit 73582c9 into RaspberryPiFoundation:goog_module Jul 30, 2021

rachel-fenichel deleted the clipboard branch July 30, 2021 18:10

rachel-fenichel mentioned this pull request Jul 30, 2021

Review contents of blockly.js #5208

Closed

33 tasks

cpcallen mentioned this pull request Aug 5, 2021

Getters for copy information #4600

Closed

Move clipboard functions to a separate namespace #5237

Move clipboard functions to a separate namespace #5237

Uh oh!

Conversation

rachel-fenichel commented Jul 28, 2021

Uh oh!

alschmiedt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rachel-fenichel commented Jul 29, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BeksOmega commented Jul 29, 2021

Uh oh!

rachel-fenichel commented Jul 30, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants