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: Change the truthy tests of width and height in WorkspaceSvg.setCachedParentSvgSize #5997

Merged
merged 1 commit into from
Mar 15, 2022

Conversation

markfinn
Copy link
Contributor

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide

The details

Resolves

fixes #5930 and possibly #5404

Proposed Changes

Change the truthy tests of width and height in WorkspaceSvg.setCachedParentSvgSize to actual comparisons with null/undefined so that zero value can be saved into the cache

Behavior Before Change

If the div containing the Blockly workspace is hidden and Blockly.svgResize is called you end up in a state where the svg is forced to zero size and it will never some back.

The cause is that during the hidden svgResize WorkspaceSvg.setCachedParentSvgSize fails to record the zero size as noted here #5404 (comment), but the SVG is still made 0x0. Later when the div is un-hidden svgResize doesn't see that the size is growing since the cached value is bad, so the SVG is not set back to a visible size.

The same as what seemed to be noticed in #5404 (comment): change the truthy tests of width and height in WorkspaceSvg.setCachedParentSvgSize to actual comparisons with null so that zero value can be saved into the cache.

Behavior After Change

Zero sizes are saved in to the cache so that they can be noticed later in a comparison when non-zero values become active. Falsey values previously caused entries to not be recorded, but the actual use case is that null/undefined values should not be recorded. Zero does not appear to have been an intentional "no update" trigger.

Reason for Changes

To allow hiding and unhiding of the blockly div to work correctly.

Test Coverage

  • Desktop Chrome
    I've been using this change in chrome for several weeks

Documentation

none

Additional Information

none

…achedParentSvgSize to actual comparisons with null so that zero value can be saved into the cache

  fixes google#5930 and possibly google#5404
@markfinn markfinn requested a review from a team as a code owner March 15, 2022 13:07
@google-cla
Copy link

google-cla bot commented Mar 15, 2022

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).

For more information, open the CLA check for this pull request.

Copy link
Contributor

@alschmiedt alschmiedt left a comment

Choose a reason for hiding this comment

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

lgtm, thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants