On Wed, 15 Mar 2023 18:18:45 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> Issue was observed because even when graphic height was less than text >> height of the Label, graphic height was considered while calculating the >> baseline offset. This was shifting the baseline offset and resulted in >> misalignment. >> >> Updated `computeBaselineOffset` to exclude graphic height from baseline >> offset calculation when graphic height is more than text height. >> >> Added unit test to validate the fix. > > modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/LabelSkinTest.java > line 2102: > >> 2100: >> ********************************************************************/ >> 2101: >> 2102: //Test for JDK-8172849 > > minor: I would have used a javadoc comment instead of the big comment block > /** Test for JDK-8172849 */ Yes javadoc comment would look better. I used the comment block to keep it consistent with the other comments present in the same file. I think if changing, then better to change all the comment blocks in the file. Please let me know your thoughts. > modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/LabelSkinTest.java > line 2136: > >> 2134: * >> * >> 2135: >> *******************************************************************************/ >> 2136: > > Minor: I would have used a javadoc explaining the purpose of this class > instead of generic "Helper classes". Same comment as above. In addition to that, yes, updating the comment with actual purpose of the class would be better. I will update that once we decide if all the other comment blocks also should be changed or not. Since there was no comment for this class, I added the comment so that it gets separated from "Tests for bug reports" tests. ------------- PR: https://git.openjdk.org/jfx/pull/1059
