On Thu, 23 Feb 2023 08:52:19 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> Karthik P K has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix text and prompt alignment issue
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TextFieldSkin.java
> line 805:
>
>> 803: // appear at the left of the centered prompt.
>> 804: newX = midPoint -
>> promptNode.getLayoutBounds().getWidth() / 2;
>> 805: if (newX > 0) {
>
> Let's say `caretWidth / 2` is 5, and that would be position we would show the
> prompt text if it doesn't fit.
>
> If the `newX` calculation is less than 5, but greater than 0, the prompt text
> would be further to the left than you'd expect.
>
> So I think this line should be:
>
> Suggestion:
>
> if (newX > caretWidth / 2) {
>
>
> Probably best to put that in a local variable.
this is undocumented, I think, but the caret path can be one of the three
things:
1. a single point (MoveTo, I think)
2. a single line - MoveTo followed by a LineTo
3. two separate lines (split caret) - MoveTo, LineTo, MoveTo, LineTo (split
caret is explicitly ignored on line 252)
One would think that the caret width can be changed by CSS, but it is not
easy. The caretPath on TextInputControl:184 has no CSS class name set, so the
width cannot be changed trivially via a stylesheet. I suppose the application
code can dig the internals looking for paths and setting widths manually, but
this would be a highly unreliable move.
So I think this caretWidth can be other than 1.0 (line 233) for the purposes of
this PR.
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TextFieldSkin.java
> line 818:
>
>> 816: } else if (newX < 0 && oldX > 1) {
>> 817: textTranslateX.set(caretWidth / 2);
>> 818: }
>
> I get the impression this code also needs to use `caretWidth / 2` instead of
> `0` and `1`.
>
> ie:
>
> Suggestion:
>
> // Update if there is space on the right
> if (newX + textNodeWidth <= textRight.get() - caretWidth / 2)
> {
> textTranslateX.set(newX);
> } else if (newX < caretWidth / 2 && oldX > caretWidth / 2) {
> textTranslateX.set(caretWidth / 2);
> }
>
>
> It would only work for very narrow caret's otherwise, and for wide caret's it
> would have some unexpected positioning in the edge case where text almost
> fits.
I would also recommend testing the code on Windows with the screen scale set to
225%, as it might show issues related to fractional scale.
-------------
PR: https://git.openjdk.org/jfx/pull/980
