-
Notifications
You must be signed in to change notification settings - Fork 6.8k
fix(cdk/text-field): fixed auto-size line-height bug at 110% zoom #32611
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
base: main
Are you sure you want to change the base?
fix(cdk/text-field): fixed auto-size line-height bug at 110% zoom #32611
Conversation
src/cdk/text-field/autosize.ts
Outdated
| // The measuring class includes a 2px padding to workaround an issue with Chrome, | ||
| // so we account for that extra space here by subtracting 4 (2px top + 2px bottom). | ||
| const scrollHeight = element.scrollHeight - 4; | ||
| // so we account for that extra space here. We subtract 3.5 (instead of 4) to account for |
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.
nit: Can we include that it's (2px top + 2px bottom) so there is more context.
| // - During measurement, the measuring class adds 4px padding; if we had an extra 0.5px in | ||
| // layout but it gets truncated, the value we read can be 1px smaller than the "real" need. | ||
| const scrollHeightDuringMeasurement = Math.floor(actualNeededScrollHeight + 4.5); |
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.
Where is the 0.5 px coming from the layout ? Is this just simulating the extra 110% browser zoom? And it's just an example right?
And I am also not sure if I understand that it can be 1px smaller than the "real" need. Isn't it 0.5px smaller? Or maybe you mean it can be UP to 1px smaller than the real need - since it rounds down.
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.
I am not sure either 😭. I asked cursor to make a regression test for it (because of the contributing guidelines) and this is what it came up for it after a bunch of attempts. I do know that this solution works, why it works and I can provide screenshot proof and mathematical proof why it works.
The addition of 0.5 does not really add 0.5, it is 0.5 to be more palatable, internally, the 0.5 is rounded up to get us an extra pixel of scroll height.
Fix scrollbar appearing with decimal line-height values in textarea autosize
Fixes #32178
Problem
When using a
<textarea>withcdkTextareaAutosizeand a decimalline-heightvalue (e.g.,1.15), a vertical scrollbar appears even when the content fits within the view. This issue occurs at certain browser zoom levels (e.g., 110%) where fractional pixel values get truncated by the browser.The root cause is in the
_measureScrollHeight()method, which subtracts4from the measuredscrollHeightto account for the 2px top and bottom padding added by the measuring class. However, when the browser truncates fractional pixels (e.g.,20.4pxbecomes20px), subtracting4can result in a height that's slightly less than what's actually needed, causing a scrollbar to appear.Solution
Changed the subtraction value from
4to3.5in_measureScrollHeight(). This provides a small margin that accounts for fractional pixel truncation, preventing scrollbars from appearing when using decimal line-height values.Changes
src/cdk/text-field/autosize.ts: Changed subtraction value from4to3.5in_measureScrollHeight()methodsrc/cdk/text-field/autosize.spec.ts: Added test'should not show scrollbar with decimal line-height values at 110% zoom'that verifies the fix and would fail if the value was4Testing
4, ensuring the fix is properly validatedImpact