Re: RFR: 8328577: Toolbar's overflow button overlaps the items [v2]

2024-03-28 Thread eduardsdv
On Thu, 28 Mar 2024 18:53:00 GMT, eduardsdv  wrote:

>> This change fixes the calculation of which nodes go to the toolbar and which 
>> go to the overflow menu.
>> It is now determined before the nodes are removed from the scene graph.
>> This is important because the values returned by 
>> ``Node.prefWidth(..)``/``Node.prefHeight(..)`` may depend on whether the 
>> node is added to the scene graph.
>> 
>> Furthermore I corrected the ``hasOveflow`` value passed to the 
>> ``organizeOverflow(double, boolean)`` in ``correctOverflow(double)``.
>
> eduardsdv has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   JDK-8328577: Enforce the overflowed nodes are added to the scene even if 
> the overflow menu is not visible

Thanks for finding it. I have reworked my solution. 

It is indeed necessary to add the overflow nodes to the scene of the popup and 
reapply the CSS so that the ``prefWidth(..)``/``prefHeight(..)`` return correct 
values, even if the nodes are no longer in the toolbar.

I will try to create a unit test that ensures the bug does not occur again.

-

PR Comment: https://git.openjdk.org/jfx/pull/1434#issuecomment-2025915810


Re: RFR: 8328577: Toolbar's overflow button overlaps the items [v2]

2024-03-28 Thread eduardsdv
> This change fixes the calculation of which nodes go to the toolbar and which 
> go to the overflow menu.
> It is now determined before the nodes are removed from the scene graph.
> This is important because the values returned by 
> ``Node.prefWidth(..)``/``Node.prefHeight(..)`` may depend on whether the node 
> is added to the scene graph.
> 
> Furthermore I corrected the ``hasOveflow`` value passed to the 
> ``organizeOverflow(double, boolean)`` in ``correctOverflow(double)``.

eduardsdv has updated the pull request incrementally with one additional commit 
since the last revision:

  JDK-8328577: Enforce the overflowed nodes are added to the scene even if the 
overflow menu is not visible

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/1434/files
  - new: https://git.openjdk.org/jfx/pull/1434/files/b7e25d53..82994291

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx=1434=01
 - incr: https://webrevs.openjdk.org/?repo=jfx=1434=00-01

  Stats: 69 lines in 1 file changed: 21 ins; 22 del; 26 mod
  Patch: https://git.openjdk.org/jfx/pull/1434.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1434/head:pull/1434

PR: https://git.openjdk.org/jfx/pull/1434