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

feat!: bubble ownership #7071

Merged
merged 13 commits into from
May 12, 2023
Merged

Conversation

BeksOmega
Copy link
Collaborator

@BeksOmega BeksOmega commented May 10, 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 #7061

Proposed Changes

Refactors the bubble so that it owns its own view elements, instead of being passed them in the constructor.

Reason for Changes

Clear separation between models and views.

Test Coverage

N/A

Documentation

N/A

Additional Information

Things may have to change as I subclass this to create the bubble subclasses, but I think this is basically gtg!

Breaking changes / updating / upgrading

This change is unlikely to affect you unless you are monkey patching Blockly to support custom icons. In this case, we recommend checking out the docs for creating custom bubbles, and bringing your implementation back to mainline based on those =)

If you were using the Blockly.Bubble class it is now located at Blockly.bubbles.Bubble. The blockly migration script can automatically perform this rename for you.

@BeksOmega BeksOmega requested a review from a team as a code owner May 10, 2023 22:54
@BeksOmega BeksOmega requested a review from maribethb May 10, 2023 22:54
@github-actions github-actions bot added the PR: feature Adds a feature label May 10, 2023
core/bubbles/bubble.ts Outdated Show resolved Hide resolved
core/bubbles/bubble.ts Outdated Show resolved Hide resolved
core/blockly.ts Show resolved Hide resolved
@BeksOmega BeksOmega force-pushed the feat/bubble-ownership branch from 7479ba5 to e284cea Compare May 11, 2023 17:34
@BeksOmega BeksOmega mentioned this pull request May 11, 2023
4 tasks
core/blockly.ts Show resolved Hide resolved
core/bubbles/bubble.ts Outdated Show resolved Hide resolved
core/utils/rect.ts Outdated Show resolved Hide resolved
@BeksOmega BeksOmega force-pushed the feat/bubble-ownership branch from 4296d87 to 64463c0 Compare May 12, 2023 16:21
@BeksOmega BeksOmega enabled auto-merge (squash) May 12, 2023 16:21
@BeksOmega BeksOmega merged commit 794c045 into google:develop May 12, 2023
@BeksOmega BeksOmega mentioned this pull request Jun 20, 2023
4 tasks
@BeksOmega BeksOmega changed the title feat: bubble ownership feat!: bubble ownership Jun 20, 2023
@github-actions github-actions bot added breaking change Used to mark a PR or issue that changes our public APIs. PR: feature Adds a feature and removed PR: feature Adds a feature labels Jun 20, 2023
@github-actions github-actions bot added breaking change Used to mark a PR or issue that changes our public APIs. PR: feature Adds a feature and removed breaking change Used to mark a PR or issue that changes our public APIs. labels Jun 20, 2023
@BeksOmega BeksOmega deleted the feat/bubble-ownership branch May 14, 2024 16:27
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: feature Adds a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor the built-in bubble class to define its own contents
2 participants