On Wed, 18 Jan 2023 06:56:57 GMT, Karthik P K <k...@openjdk.org> wrote:
>> Ignore text condition was not considered in `computePrefHeight` method while >> calculating the Label height. >> >> Added `isIgnoreText` condition in `computePrefHeight` method while >> calculating Label height. >> >> Tests are present for all the ContentDisplay types. Modified tests which >> were failing because of the new height value getting calculated. > > Karthik P K has updated the pull request incrementally with one additional > commit since the last revision: > > Use width while calling prefHeight method. Use graphicHeight instead of > multiple calls to prefHeight modules/javafx.controls/src/main/java/javafx/scene/control/skin/LabeledSkinBase.java line 402: > 400: } > 401: > 402: return height + padding; My only comment is possibly reorganize the logic to avoid unnecessary computation. For example, there is no need to compute text width (line 329) if isIgnoreText() is true; Similarly, no need to compute graphicWidth on line 333 if isIgnoreGraphic() is true. (basically, same comment as @hjohn has made earlier) What do you think, @karthikpandelu ? ------------- PR: https://git.openjdk.org/jfx/pull/996
