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!: refactor mutator icon #7115

Merged
merged 24 commits into from
Jun 2, 2023
Merged

Conversation

BeksOmega
Copy link
Collaborator

@BeksOmega BeksOmega commented May 23, 2023

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide
  • I ran npm run format and npm run lint

The details

Resolves

Fixes #7039

Proposed Changes

Refactors the mutator icon to conform to the IIcon interface.

Reason for Changes

Code consistency :D

Test Coverage

Updated tests! Also manual testing :D

Documentation

N/A

Additional Information

Dependent on #7110

Breaking changes / updating / upgrading

The Blockly.Mutator class has been renamed to Blockly.icons.MutatorIcon. If you use the blockly migration package, you can automatically rename this throughout your files by doing:

npx @blockly/migrate rename --from 9 --in-place ./path/to/my/files*

Note that you are now required to pass the block the mutator icon will live on to the mutator icon's constructor.

myBlock.addIcon(new Blockly.icons.MutatorIcon(['my_quark_block'], myBlock));

Misc methods

The updateBlockStyle method is no longer available. Updating block styles is handled automatically, so you should be able to safely remove any references to this method. If removing this causes problems for your application, please file a bug report.

The shouldIgnoreMutatorEvent method is no longer available. This method was originally intended for internal use, and should not have been public. If you were using this method, we recommend duplicating the code from here into your own application.

Deprecations

The Mutator.reconnect method has been deprecated. Use Connection.prototype.reconnect instead.

The findParentWs method has been deprecated. Use Workspace.prototype.getRootWorkspace instead.

Showing/hiding bubbles

The setVisible method has been renamed to setBubbleVisible for clarity. If you need to modify whether the bubble of the mutator icon is visible or not, you can do:

myBlock.getIcon(Blockly.icons.MutatorIcon.TYPE).setBubbleVisible(true);

The setVisible method will be removed in v11.

@github-actions github-actions bot added breaking change Used to mark a PR or issue that changes our public APIs. PR: fix Fixes a bug labels May 23, 2023
core/icons/mutator_icon.ts Outdated Show resolved Hide resolved
@BeksOmega BeksOmega force-pushed the feat/mutator-icon branch 2 times, most recently from e9e2dd0 to 001505f Compare May 24, 2023 20:20
@BeksOmega BeksOmega marked this pull request as ready for review May 24, 2023 20:48
@BeksOmega BeksOmega requested a review from a team as a code owner May 24, 2023 20:48
@BeksOmega BeksOmega requested a review from cpcallen May 24, 2023 20:48
@github-actions github-actions bot added breaking change Used to mark a PR or issue that changes our public APIs. PR: fix Fixes a bug and removed PR: fix Fixes a bug breaking change Used to mark a PR or issue that changes our public APIs. labels May 24, 2023
Copy link
Contributor

@cpcallen cpcallen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not quite finished reviewing all of mutator_icon.ts, but thought I should send you my comments so far.

core/block_svg.ts Show resolved Hide resolved
core/block_svg.ts Show resolved Hide resolved
core/bubbles/bubble.ts Outdated Show resolved Hide resolved
core/bubbles/mini_workspace_bubble.ts Show resolved Hide resolved
core/bubbles/mini_workspace_bubble.ts Show resolved Hide resolved
core/icons/mutator_icon.ts Outdated Show resolved Hide resolved
core/icons/mutator_icon.ts Show resolved Hide resolved
core/icons/mutator_icon.ts Show resolved Hide resolved
core/icons/mutator_icon.ts Show resolved Hide resolved
core/icons/mutator_icon.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@cpcallen cpcallen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not quite finished reviewing all of mutator_icon.ts, but thought I should send you my comments so far.

Copy link
Contributor

@cpcallen cpcallen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finished reviewing; had a few additional comments.

core/icons/mutator_icon.ts Outdated Show resolved Hide resolved
core/icons/mutator_icon.ts Show resolved Hide resolved
core/icons/mutator_icon.ts Outdated Show resolved Hide resolved
core/icons/mutator_icon.ts Outdated Show resolved Hide resolved
core/icons/mutator_icon.ts Outdated Show resolved Hide resolved
core/icons/mutator_icon.ts Show resolved Hide resolved
core/icons/mutator_icon.ts Outdated Show resolved Hide resolved
core/icons/mutator_icon.ts Outdated Show resolved Hide resolved
core/block_svg.ts Show resolved Hide resolved
core/icons/mutator_icon.ts Outdated Show resolved Hide resolved
core/icons/mutator_icon.ts Outdated Show resolved Hide resolved
core/icons/mutator_icon.ts Outdated Show resolved Hide resolved
core/icons/mutator_icon.ts Show resolved Hide resolved
blocks/procedures.js Outdated Show resolved Hide resolved
core/icons/icon_types.ts Outdated Show resolved Hide resolved
core/icons/mutator_icon.ts Outdated Show resolved Hide resolved
core/icons/mutator_icon.ts Show resolved Hide resolved
@BeksOmega BeksOmega mentioned this pull request Jun 1, 2023
4 tasks
@BeksOmega BeksOmega force-pushed the feat/mutator-icon branch 3 times, most recently from f53ae53 to 83a8c33 Compare June 1, 2023 14:40
Copy link
Contributor

@cpcallen cpcallen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few small nits which I will leave for you to resolve as you see fit.

core/connection.ts Outdated Show resolved Hide resolved
core/connection.ts Show resolved Hide resolved
core/workspace.ts Show resolved Hide resolved
scripts/migration/renamings.json5 Outdated Show resolved Hide resolved
@BeksOmega BeksOmega force-pushed the feat/mutator-icon branch 2 times, most recently from 55a89b0 to 9bc8e2d Compare June 2, 2023 14:34
@github-actions github-actions bot removed the PR: fix Fixes a bug label Jun 2, 2023
@BeksOmega BeksOmega force-pushed the feat/mutator-icon branch from 2392771 to 70af1a5 Compare June 2, 2023 18:28
@BeksOmega BeksOmega merged commit 2f74ce8 into google:develop Jun 2, 2023
@github-actions github-actions bot added breaking change Used to mark a PR or issue that changes our public APIs. PR: fix Fixes a bug and removed PR: fix Fixes a bug breaking change Used to mark a PR or issue that changes our public APIs. labels Jun 14, 2023
@github-actions github-actions bot added breaking change Used to mark a PR or issue that changes our public APIs. PR: fix Fixes a bug and removed PR: fix Fixes a bug breaking change Used to mark a PR or issue that changes our public APIs. labels Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Used to mark a PR or issue that changes our public APIs. PR: fix Fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor the Mutator icon to conform to the IIcon interface
2 participants