RFR: 8371859: Dialog unnecessarily invokes DialogPane.requestLayout()

[Andy Goryachev]

On Thu, 13 Nov 2025 21:24:25 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

> The implementation of `Dialog.dialogPane` adds a `ListChangeListener` to `DialogPane.getButtonTypes()`, and calls `DialogPane.requestLayout()` from the listener.
> 
> This is not needed, because the `DialogPane.createButtonBar()` method already specifies that implementers are required to observe changes of `DialogPane.getButtonTypes()`, and relayout the button bar as needed.
> 
> I've added a test to verify that changing the button types indeed requests a layout on the `DialogPane`.

The fix looks good, but (unrelated) why does this test fails:


    @Test
    public void layoutIsRequestedWhenButtonTypesChange() {
        dialogPane.getButtonTypes().clear();
        dialogPane.layout();
        assertFalse(dialogPane.isNeedsLayout());

        dialogPane.getButtonTypes().add(ButtonType.OK);
        assertTrue(dialogPane.isNeedsLayout());
        
        dialogPane.layout();
        assertFalse(dialogPane.isNeedsLayout()); // fails??
        
        dialogPane.getButtonTypes().setAll(ButtonType.OK);
        assertTrue(dialogPane.isNeedsLayout());
    }

... because it's the case when layout needs multiple pulses to get settled?

This works:


    @Test
    public void layoutIsRequestedWhenButtonTypesChange() {
        dialogPane.getButtonTypes().clear();
        dialogPane.layout();
        assertFalse(dialogPane.isNeedsLayout());

        dialogPane.getButtonTypes().add(ButtonType.OK);
        assertTrue(dialogPane.isNeedsLayout());
        
        dialogPane.layout();
        dialogPane.layout();
        assertFalse(dialogPane.isNeedsLayout()); // passes
        
        dialogPane.getButtonTypes().setAll(ButtonType.OK);
        assertTrue(dialogPane.isNeedsLayout());
    }

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1973#issuecomment-3530086506
PR Comment: https://git.openjdk.org/jfx/pull/1973#issuecomment-3530094041