Skip to content

/ jfx

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

Jump to bottom

8242548: Wrapped labeled controls using -fx-line-spacing cut text off #173

Closed
wants to merge 2 commits into from
+53 −5
Closed

8242548: Wrapped labeled controls using -fx-line-spacing cut text off #173

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
20ccd12
8242548: Honor line spacing in Labeled reflow calculation
hjohn Apr 23, 2020
8fa8f3f
Fix typo in comment
hjohn Apr 27, 2020
File filter...
Filter file types
Clear filters
Jump to…
Jump to file
Failed to load files.

Always

Just for now

2 modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/skin/Utils.java
View file
Open in desktop
@@ -429,7 +429,7 @@ public static String computeClippedWrappedText(Font font, String text, double wi
// However the calculations include the line spacing as part of a
// line's height. In order to not cut off the last line because its
// line spacing wouldn't fit, the height used for the calculation
// is increase here with the line spacing amount.
// is increased here with the line spacing amount.

height += lineSpacing;
This conversation was marked as resolved by hjohn

This comment has been minimized.

Sign in to view
@aghaisas

aghaisas Apr 22, 2020
Collaborator

Modifying 'height' parameter seems incorrect as this parameter is used in other calculations below. If needed, use a separate local variable to use 'height+lineSpacing'

This comment has been minimized.

Sign in to view
@hjohn

hjohn Apr 22, 2020
Author Collaborator

The height used needs to be changed everywhere. However, I can move this to the caller instead, with a comment explaining why:

The computeClippedWrappedText call works with full lines (height of line + line spacing), including the last line. However the wrap height does not include the line spacing for the last line. In order to avoid wrapping the last line of text even though there is sufficient space the wrap height passed to computeClippedWrappedText is increased with the line spacing so it computes the correct text.

This comment has been minimized.

Sign in to view
@aghaisas

aghaisas Apr 23, 2020
Collaborator

I see your point. I am OK with the changes that you have done.

This conversation was marked as resolved by hjohn

This comment has been minimized.

Sign in to view
@aghaisas

aghaisas Apr 22, 2020
Collaborator

This method makes a call to -
double eHeight = computeTextHeight(font, ellipsis, 0, boundsType);

What is the behavior if we make a call to the other method that takes in lineSpacing?
double eHeight = computeTextHeight(font, ellipsis, 0, lineSpacing, boundsType);

This comment has been minimized.

Sign in to view
@hjohn

hjohn Apr 22, 2020
Author Collaborator

The call only determines the ellipsis height (eHeight) to see if the space is so small that even an ellipsis wouldn't fit. For a multi-line ellipsis (if there is such a thing) we could pass in the line spacing here.
ProTip! Use n and p to navigate between commits in a pull request.