On Thu, 23 Feb 2023 16:34:11 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> 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. > I get the impression this code also needs to use `caretWidth / 2` instead of > `0` and `1`. > Here also same comment as above for the `newX < caretWidth/2` condition. But while comparing the `oldX`, `caretWidth/2` should be considered instead of 1 and also `caretWidth / 2` is subtracted from `textRight` value in the previous if condition as you suggested. These updates have been done in the code. > 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. I will test this latest code updates. ------------- PR: https://git.openjdk.org/jfx/pull/980