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: size text with computed styles even when hidden #8572

Merged
merged 2 commits into from
Sep 13, 2024

Conversation

gonfunko
Copy link
Contributor

The basics

The details

Resolves

Re-fixes #8277

Proposed Changes

Recently, code that was calling getFastTextWidth() was updated to call getTextWidth() instead, because getFastTextWidth() required knowledge of some styling attributes. Historically, these were derived from the theme/renderer, but with recent changes to move towards using CSS for this purpose, it was thought that we needed to use getTextWidth() instead, which relied on the getComputedTextLength() method of SVG text elements.

Unfortunately, getComputedTextLength() has a significant limitation: it doesn't work when the SVG text element (or any of its parents) has display: none or similar set. This could result in blocks being incorrectly sized when manipulated while not visible, e.g. gonfunko/scratch-blocks#163.

This PR modifies the implementation of getTextWidth() to instead use getComputedStyle() to dynamically calculate the CSS styles applied to an SVG text element, whether through Blockly's built-in styles, injected styles, or user agent stylesheets. This method returns a live CSSStyleDeclaration object, which allows accessing the currently-applicable value of any CSS property. AIUI, this doesn't calculate anything up front, only determining the value of properties when they are accessed, so performance should be (and appears to be) quite good. Moreover, this appears to return e.g. the font size/weight/face applied to an element even if it is hidden at the moment, and resolves the aforementioned issues.

This PR additionally removes exception handling and size estimation code that was presumably present due to an IE 10 behavior that caused getComputedTextLength() to throw for hidden elements.

@gonfunko gonfunko requested a review from a team as a code owner September 11, 2024 21:39
@github-actions github-actions bot added the PR: fix Fixes a bug label Sep 11, 2024
@gonfunko gonfunko merged commit 732bd7f into google:rc/v12.0.0 Sep 13, 2024
11 checks passed
@gonfunko gonfunko deleted the block-sizing branch September 13, 2024 18:40
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
Development

Successfully merging this pull request may close these issues.

2 participants