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

Correctly clip the TextBox #1775

Merged
merged 1 commit into from
May 12, 2021
Merged

Correctly clip the TextBox #1775

merged 1 commit into from
May 12, 2021

Conversation

CryZe
Copy link
Contributor

@CryZe CryZe commented May 12, 2021

By adjusting only the x component of the size before turning it into a rectangle, the right side of the rectangle gets moved to the left, which allowed the text to peak out on the right side. Inset already seems to do what subtracting the border width there was possibly intended to do, so just using the unadjusted size and then later insetting, resolves the problem entirely.

Here for reference the bug that this resolves:

https://i.imgur.com/Kvr1n4T.png

By adjusting only the x component of the size before turning it into a
rectangle, the right side of the rectangle gets moved, which allowed the
text to peak out on the right side. Inset already seems to do what
subtracting the border width there was possibly intended to do, so just
using the unadjusted size and then later insetting, resolves the problem
entirely.
Copy link
Collaborator

@maan2003 maan2003 left a comment

Choose a reason for hiding this comment

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

Looks Good. Thanks!

@maan2003 maan2003 added the S-ready PR is ready to merge label May 12, 2021
@cmyr
Copy link
Member

cmyr commented May 12, 2021

cool, I think this was a relic of an earlier version, where the padding was calculated and applied manually.

@cmyr cmyr merged commit 6f070f1 into linebender:master May 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-ready PR is ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants