RFR: 8172849: Non-intuitive baseline alignment for labeled controls with graphics
Karthik P K
kpk at openjdk.org
Thu Mar 16 06:34:37 UTC 2023
On Wed, 15 Mar 2023 18:18:45 GMT, Andy Goryachev <angorya at 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
More information about the openjfx-dev
mailing list