-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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(blocks): boolean variable block wrong shape for zelos #7335
fix(blocks): boolean variable block wrong shape for zelos #7335
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
43b4ed2
to
533230a
Compare
533230a
to
9df7f9d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for finding this bug and submitting a fix :D The change looks great, but I think there's actually a more general place we can put it!
blocks/variables_dynamic.ts
Outdated
if (this.rendered) { | ||
(this as BlockSvg).queueRender(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of having this inside the individual block definition, you can override setCheck
in the rendered connection class, and call queueRender
there! This way whenever a connection check gets updated on any block, we'll be sure the shapes of the connections get updated =)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for taking the time to review my PR and providing your feedback. I appreciate your suggestions. updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sweet, LGTM! I'll get this merged after CI Passes =) Thank you for your hard work on this!!
The basics
npm run format
andnpm run lint
The details
Resolves
Fixes #7334
Proposed Changes
Behavior Before Change
Behavior After Change
Reason for Changes
I think it's a bug 🤔️
Test Coverage
Documentation
Additional Information
Is there a better way than rerender?