On Tue, 17 Jun 2025 22:48:03 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> Andy Goryachev has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 56 commits: >> >> - Merge remote-tracking branch 'origin/master' into 8314482.tab.stops >> - cleanup >> - api >> - Merge remote-tracking branch 'origin/master' into 8314482.tab.stops >> - cleanup >> - Merge remote-tracking branch 'origin/master' into 8314482.tab.stops >> - seems to work >> - wired >> - moved >> - Merge remote-tracking branch 'origin/master' into 8314482.tab.stops >> - ... and 46 more: https://git.openjdk.org/jfx/compare/1ea980ea...7acc55db > > modules/javafx.graphics/src/main/java/javafx/scene/text/TabStop.java line 35: > >> 33: * @since 999 TODO >> 34: */ >> 35: public class TabStop { > > Should this be a record? If not, it should at least be a final class. No and no. The application code or the custom component should be able to extend the TabStop class to supply additional options. > modules/javafx.graphics/src/main/java/javafx/scene/text/TabStop.java line 41: > >> 39: * Constructs a new tab stop with the specified position. >> 40: * >> 41: * @param position the position in pixels > > Is pixels the right unit here? Should it be points instead? Or should it > allow for the specification of units so that you could specify it in "em"s? > > @prrace might want to weigh in on this. Using the same units used everywhere else (text layout, padding, border, etc.) I think any conversion should be the responsibility of the application code. > modules/javafx.graphics/src/main/java/javafx/scene/text/TabStopPolicy.java > line 39: > >> 37: * @since 999 TODO >> 38: */ >> 39: public class TabStopPolicy { > > Is there a reason this class is not final? It probably should be. good point, changed to `final`. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1744#discussion_r2154840273 PR Review Comment: https://git.openjdk.org/jfx/pull/1744#discussion_r2154836049 PR Review Comment: https://git.openjdk.org/jfx/pull/1744#discussion_r2154841884
