RFR: 8364049: ToolBar shows overflow menu with fractional scale [v2]
Michael Strauß
mstrauss at openjdk.org
Wed Jul 30 18:09:01 UTC 2025
On Wed, 30 Jul 2025 17:42:54 GMT, Andy Goryachev <angorya at openjdk.org> wrote:
>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/ToolBarSkin.java line 692:
>>
>>> 690: */
>>> 691: private int getOverflowNodeIndex(double length) {
>>> 692: if (getSkinnable().getOrientation() == Orientation.VERTICAL) {
>>
>> Snapping the input argument of a method like `getOverflowNodeIndex` doesn't seem right to me, because the length is computed incorrectly in the first place. Have you considered fixing the snapping in `getToolbarLength`? This also solves the problem.
>
> Yes, but I wanted to do a very localized fix, to be included in jfx25. There are many places where we should be more careful with snapping mentioned in description, and I suspect even more elsewhere.
>
> I am still not sure what would be the best approach to address them all. We could probably create an umbrella task and fix individual controls and containers (maybe even start with containers, in continuation of John's work with HBox and VBox).
>
> I think this PR is still good, because a) it provides a fix for the specific issue and b) minimizes regression, but I agree with you that a comprehensive fix is needed.
>
> What do you think?
I understand that you want this to be a localized fix. I'm just proposing to change `getToolbarLength` instead, which is even less changed code than your change in `getOverflowNodeIndex`, and seems to be addressing the source of the incorrect length instead.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1856#discussion_r2243493210
More information about the openjfx-dev
mailing list