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

Text width measuring does not respect CSS. #5871

Closed
NeilFraser opened this issue Jan 12, 2022 · 4 comments
Closed

Text width measuring does not respect CSS. #5871

NeilFraser opened this issue Jan 12, 2022 · 4 comments
Labels
component: rendering issue: bug Describes why the code or behaviour is wrong

Comments

@NeilFraser
Copy link
Contributor

FieldLabels have an optional class parameter (which interestingly is not mentioned in the docs, but is listed in the reference). This allows one to change the font/size/colour and other attributes of the text.

However, changes made by a class to the label does not affect the computed width. Blockly.utils.dom.getTextWidth does a perfectly good job, but it has largely been replaced by Blockly.utils.dom.getFastTextWidth. Whereas the former measures the actual onscreen text using the built-in getComputedTextLength function, the latter draws the text onto a canvas and measures the result. Nothing drawn on a canvas is affected by CSS class rules.

The result is that (for example) if one adds some monotype text to a block, the text spills off the edge of the block:
Screen Shot 2022-01-11 at 23 45 07

Changing text colour is about the only safe action one can perform using a CSS class.

This bug surfaced as a result of PR #5868, but it's been lurking in the shadows for years ever since getFastTextWidth was created.

@NeilFraser NeilFraser added issue: bug Describes why the code or behaviour is wrong component: rendering issue: triage Issues awaiting triage by a Blockly team member labels Jan 12, 2022
@NeilFraser NeilFraser self-assigned this Jan 12, 2022
@BeksOmega
Copy link
Collaborator

which interestingly is not mentioned in the docs

Technically is mentioned in the docs :P "The label field takes in an optional value, and an optional css class string." But yeeee you're right, it should probably be in the example code, as well as in the description.

Also I think this is a dupe of #2694 ? Not sure which is better to close though.

@NeilFraser
Copy link
Contributor Author

I think #2694 is a superset of this issue, not a duplicate. Its problem is mainly a complete lack of height info, whereas this issue only deals with incorrect width info.

Obtaining height info is nearly impossible, whereas correcting the width info appears to be feasible. Thus I expect we can quickly resolve this issue, while leaving #2694 open.

@gonfunko gonfunko removed the issue: triage Issues awaiting triage by a Blockly team member label Jan 14, 2022
@BeksOmega BeksOmega assigned BeksOmega and unassigned NeilFraser Jan 18, 2023
@BeksOmega BeksOmega removed their assignment Mar 1, 2023
nwinter added a commit to codecombat/codecombat that referenced this issue Feb 9, 2024
This extends our blockly monkey patch to use the slower, more accurate
way of figuring out how big the comment text field should be, as
detailed in google/blockly#5871.
@sahilrastogi94
Copy link

Is this still being looked at?

@gonfunko
Copy link
Contributor

This seems to have been fixed by #8572.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: rendering issue: bug Describes why the code or behaviour is wrong
Projects
None yet
Development

No branches or pull requests

4 participants