Skip to content

Commit

Permalink
8242548: Wrapped labeled controls using -fx-line-spacing cut text off
Browse files Browse the repository at this point in the history
Reviewed-by: aghaisas, kcr
  • Loading branch information
hjohn authored and aghaisas committed May 13, 2020
1 parent 435671e commit 7b06190
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 5 deletions.
Expand Up @@ -417,17 +417,27 @@ public static String computeClippedText(Font font, String text, double width,
}

public static String computeClippedWrappedText(Font font, String text, double width,
double height, OverrunStyle truncationStyle,
double height, double lineSpacing, OverrunStyle truncationStyle,
String ellipsisString, TextBoundsType boundsType) {
if (font == null) {
throw new IllegalArgumentException("Must specify a font");
}

// The height given does not need to include the line spacing after
// the last line to be able to render that last line correctly.
//
// 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 increased here with the line spacing amount.

height += lineSpacing;

String ellipsis = (truncationStyle == CLIP) ? "" : ellipsisString;
int eLen = ellipsis.length();
// Do this before using helper, as it's not reentrant.
double eWidth = computeTextWidth(font, ellipsis, 0);
double eHeight = computeTextHeight(font, ellipsis, 0, boundsType);
double eHeight = computeTextHeight(font, ellipsis, 0, lineSpacing, boundsType);

if (width < eWidth || height < eHeight) {
// The ellipsis doesn't fit.
Expand All @@ -438,7 +448,7 @@ public static String computeClippedWrappedText(Font font, String text, double wi
helper.setFont(font);
helper.setWrappingWidth((int)Math.ceil(width));
helper.setBoundsType(boundsType);
helper.setLineSpacing(0);
helper.setLineSpacing(lineSpacing);

boolean leading = (truncationStyle == LEADING_ELLIPSIS ||
truncationStyle == LEADING_WORD_ELLIPSIS);
Expand Down
Expand Up @@ -1107,7 +1107,7 @@ private void updateDisplayedText(double w, double h) {
String ellipsisString = labeled.getEllipsisString();

if (labeled.isWrapText()) {
result = Utils.computeClippedWrappedText(font, s, wrapWidth, wrapHeight, truncationStyle, ellipsisString, text.getBoundsType());
result = Utils.computeClippedWrappedText(font, s, wrapWidth, wrapHeight, labeled.getLineSpacing(), truncationStyle, ellipsisString, text.getBoundsType());
} else if (multiline) {
StringBuilder sb = new StringBuilder();

Expand Down
Expand Up @@ -136,6 +136,12 @@ public class LabelSkinTest {
assertTrue(skin.propertyChanged);
}

@Test public void lineSpacingChangesOnLabelShouldInvoke_handleControlPropertyChanged() {
skin.addWatchedProperty(label.lineSpacingProperty());
label.setLineSpacing(1.0);
assertTrue(skin.propertyChanged);
}

@Test public void textChangesOnLabelShouldInvoke_handleControlPropertyChanged() {
skin.addWatchedProperty(label.textProperty());
label.setText("Bust my buffers!");
Expand Down Expand Up @@ -279,6 +285,15 @@ public class LabelSkinTest {
assertFalse(skin.get_invalidText());
}

@Test public void lineSpacingChangesOnLabelShouldInvalidateLayoutAndDisplayText() {
label.layout();
skin.updateDisplayedText();

label.setLineSpacing(1.0);
assertTrue(label.isNeedsLayout());
assertTrue(skin.get_invalidText());
}

@Test public void textChangesOnLabelShouldInvalidateLayoutAndDisplayTextAndTextWidth() {
label.layout();
skin.updateDisplayedText();
Expand Down Expand Up @@ -1145,6 +1160,26 @@ public class LabelSkinTest {
assertTrue(height >= singleLineHeight * 5);
}

@Test public void whenTextHasNewlinesAndPositiveLineSpacing_computePrefHeight_IncludesTheMultipleLinesAndLineSpacingInThePrefHeight() {
label.setLineSpacing(2);
label.setText("This\nis a test\nof the emergency\nbroadcast system.\nThis is only a test");
label.setPadding(new Insets(0, 0, 0, 0));
final double singleLineHeight = Utils.computeTextHeight(label.getFont(), " ", 0, text.getBoundsType());
final double expectedHeight = singleLineHeight * 5 + label.getLineSpacing() * 5 - label.getLineSpacing();
final double height = label.prefHeight(-1);
assertEquals(expectedHeight, height, 0);
}

@Test public void whenTextHasNewlinesAndNegativeLineSpacing_computePrefHeight_IncludesTheMultipleLinesAndLineSpacingInThePrefHeight() {
label.setLineSpacing(-2);
label.setText("This\nis a test\nof the emergency\nbroadcast system.\nThis is only a test");
label.setPadding(new Insets(0, 0, 0, 0));
final double singleLineHeight = Utils.computeTextHeight(label.getFont(), " ", 0, text.getBoundsType());
final double expectedHeight = singleLineHeight * 5 + label.getLineSpacing() * 5 - label.getLineSpacing();
final double height = label.prefHeight(-1);
assertEquals(expectedHeight, height, 0);
}

@Test public void whenTextHasNewlinesAfterPreviousComputationOf_computePrefHeight_IncludesTheMultipleLinesInThePrefHeight() {
label.setText("This is a test");
final double oldPrefHeight = label.prefHeight(-1);
Expand Down
Expand Up @@ -48,6 +48,7 @@ public boolean setContent(TextSpan[] spans) {
private Font font;
private int tabSize = DEFAULT_TAB_SIZE;
private int nullFontSize = 0;
private float spacing;

@Override
public boolean setContent(String text, Object font) {
Expand All @@ -69,6 +70,8 @@ public boolean setWrapWidth(float wrapWidth) {

@Override
public boolean setLineSpacing(float spacing) {
this.spacing = spacing;

return true;
}

Expand All @@ -92,7 +95,7 @@ public BaseBounds getBounds(TextSpan filter, BaseBounds bounds) {
final double fontSize = (font == null ? nullFontSize : ((Font)font).getSize());
final String[] lines = getText().split("\n");
double width = 0.0;
double height = fontSize * lines.length;
double height = fontSize * lines.length + spacing * (lines.length - 1);
for (String line : lines) {
final int length;
if (line.contains("\t")) {
Expand Down

0 comments on commit 7b06190

Please sign in to comment.