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: remove forced rerender from mutator #6918

Merged
merged 1 commit into from
Apr 5, 2023

Conversation

BeksOmega
Copy link
Collaborator

@BeksOmega BeksOmega commented Mar 22, 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 #3511
Fixes #6953
Work on #4288

Proposed Changes

Removes the batch rendering from the mutator logic - since we now have actual batch rendering that doesn't rely on mangling the rendered property!

Reason for Changes

Code simplification.

Test Coverage

Manually tested that #4175 and #3458 do not reproduce.

Tested that mutating continues to work.

Documentation

N/A

Additional Information

While I do trust this code, I don't trust it enough to put it into the imminently approaching release. So I'm going to put this up as a draft.

@github-actions github-actions bot added the PR: fix Fixes a bug label Mar 22, 2023
@BeksOmega BeksOmega marked this pull request as ready for review April 4, 2023 15:09
@BeksOmega BeksOmega requested a review from a team as a code owner April 4, 2023 15:09
@maribethb
Copy link
Contributor

Fixes #6953 correct?

@github-actions github-actions bot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels Apr 5, 2023
@BeksOmega
Copy link
Collaborator Author

Fixes #6953 correct?

Editted PR description!

@BeksOmega BeksOmega merged commit 25c6d47 into google:develop Apr 5, 2023
@EricAlm
Copy link

EricAlm commented Apr 6, 2023

Sorry, what should I do to have the solution to #6953 ?

@cpcallen
Copy link
Contributor

cpcallen commented Apr 6, 2023

While I do trust this code, I don't trust it enough to put it into the imminently approaching release. So I'm going to put this up as a draft.

I applaud this instinct, but it looks like in this particular case you should have had more faith in your coding!

@cpcallen
Copy link
Contributor

cpcallen commented Apr 6, 2023

Sorry, what should I do to have the solution to #6953 ?

The easiest thing is to wait for the next release of Blockly.

If this (annoying but largely cosmetic) bug can't wait, you can build Blockly yourself from the develop branch. Unfortunately the "Building Blockly" instructions on our website are out-of-date (they'll get fixed soon—that's pretty much the next item on my to-do list) but for now if you clone the google/blockly repository and then npm install && npm run package to build Blockly. After that, if you (cd dist && npm link) within Blockly, then npm link blockly in your own project, your project will (temporarily) be linked to the version of Blockly built in dist/ (instead of the version obtained from the NPM registry).

BeksOmega added a commit to BeksOmega/blockly that referenced this pull request Apr 6, 2023
@BeksOmega BeksOmega mentioned this pull request Apr 6, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: fix Fixes a bug
Projects
None yet
6 participants