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

Use a line-height of 1.3x for the default text styles #587

Merged
merged 2 commits into from
Sep 9, 2024

Conversation

rock3r
Copy link
Collaborator

@rock3r rock3r commented Sep 7, 2024

It looks like Swing uses a line height that is 1.3 times the font size, but that is not expressed anywhere I could find. I got the value by experimentation and pixel counting, like a proper DF review would 🙃 Using this value seems to work at both default and h[0-5] text sizes.

Having a non-default line height is a bit of a pain for users, since they need to remember updating the line height too when they customize the font size in a text style; however, we can't do anything about it, and in general, folks should stick to the predefined text styles.

To help, we now provide a TextStyle.copyWithSize() function that automatically sets the correct line height, making this slightly less painful.

Comparison between Swing (top) and Compose (bottom):

Before After
image image

The difference is subtle, but it's there. I pixel counted and they match exactly with Swing (with a 1px tolerance on bigger text sizes) across all predefined text styles.

This closes #585

@rock3r rock3r requested a review from c5inco September 7, 2024 09:17
@rock3r rock3r self-assigned this Sep 7, 2024
@rock3r rock3r added the consistency Our UI presentation is not consistent with IJ label Sep 7, 2024
@rock3r rock3r requested a review from hamen September 7, 2024 09:17
It looks like Swing uses a line height that is 1.3 times the font size,
but that is not expressed anywhere I could find. I got the value by
experimentation and pixel counting, like a proper DF review would 🙃
Using this value seems to work at both default and h[0-5] text sizes.

Having a non-default line height is a bit of a pain for users, since
they need to remember updating the line height too when they customize
the font size in a text style; however, we can't do anything about it,
and in general, folks should stick to the predefined text styles.

To help, we now provide a `TextStyle.copyWithSize()` function that
automatically sets the correct line height, making this slightly less
painful.
@rock3r rock3r force-pushed the default-text-style-line-height branch from b55ed7a to 2a74266 Compare September 7, 2024 09:23
Copy link
Collaborator

@c5inco c5inco left a comment

Choose a reason for hiding this comment

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

Left one comment on the multiplier in the bridge

Also solves the issue that we were ignoring the passed in line height;
now we use the default only as fallback if a height is not passed in.
@rock3r rock3r enabled auto-merge (squash) September 7, 2024 09:48
@rock3r rock3r merged commit 0c0680e into main Sep 9, 2024
1 check passed
@rock3r rock3r deleted the default-text-style-line-height branch September 9, 2024 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consistency Our UI presentation is not consistent with IJ
Projects
None yet
Development

Successfully merging this pull request may close these issues.

defaultTextStyle lineheight does't match IDE
3 participants