8092379: GridPane should not render extra gaps when entire rows or columns are unmanaged

[hjohn]

This fixes a long standing bug with GridPane where unmanaged nodes would still result in gaps being added between rows/columns even though nothing is rendered there. Take the following grid where dashes and pipes represent the vgaps and hgaps:

	0		1
0	A	|	B
	-	|	-
1	C	|	D
	-	|	-
2	E	|	F

Now, when both C and D are set to unmanaged (and invisible) the grid will still render the gaps:

	0		1
0	A	|	B
	-	|	-
1		|
	-	|	-
2	E	|	F

Instead it should collapse the gap between row 0 and 2:

	0		1
0	A	|	B
	-	|	-
2	E	|	F

There are at least two relevant issues in JBS:

• A request to let the user show and hide rows and columns: https://bugs.openjdk.org/browse/JDK-8136901
  • This can now be done by setting the relevant row/columns items to unmanaged without having to restructure the grid (assuming the complaint was that otherwise there'd be visible extra gaps for each hidden row)
• A request for another mode to skip gaps when entire rows/columns are unmanaged: https://bugs.openjdk.org/browse/JDK-8092379
  • This should not be a mode, but the standard, as unmanaged nodes should not affect the outcome of the grid layout

Screenshots

Simple application which can hide row 3:

Correct behavior when row 3 is hidden:

Behavior before this fix (note the double gap and extra grid line):

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

• JDK-8092379: GridPane should not render extra gaps when entire rows or columns are unmanaged (Enhancement - P4)

Reviewers

• Andy Goryachev (@andy-goryachev-oracle - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1990/head:pull/1990
$ git checkout pull/1990

Update a local copy of the PR:
$ git checkout pull/1990
$ git pull https://git.openjdk.org/jfx.git pull/1990/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1990

View PR using the GUI difftool:
$ git pr show -t 1990

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1990.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back jhendrikx! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[mlbridge]

Webrevs

• 03: Full - Incremental (69eedce5)
• 02: Full - Incremental (324eb9c4)
• 01: Full - Incremental (9f2f95df)
• 00: Full (8155fc61)

[openjdk]

@hjohn Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

[andy-goryachev-oracle]

/reviewers 2

[openjdk]

@andy-goryachev-oracle
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

[andy-goryachev-oracle]

Looks good.

I've updated the standalone Monkey Tester to add a couple of layout scenarios to the GridPane page.

[andy-goryachev-oracle]

I don't know why github shows this as a change.
Unused setMultiSize() has been removed which is ok.

[hjohn]

It's confusing the start of the diff I think :)

[andy-goryachev-oracle]

unrelated change, and probably gets inlined anyway.

[hjohn]

Yeah, could remove, I just made the change because it was giving a raw type warning.

[andy-goryachev-oracle]

I turned the raw warnings off, the project has thousands of those...

[hjohn]

Looks good.

I've updated the standalone Monkey Tester to add a couple of layout scenarios to the GridPane page.

Thanks, GridPane is quite a useful layout, but if you want to hide a row or column, it is a huge pain currently (unless you use hgap/vgap of 0) as you need to physically renumber your rows/columns to collapse the extra gaps...

[andy-goryachev-oracle]

huge pain, yes, that's why I am still working on https://github.com/andy-goryachev/FxDock/blob/master/doc/CPane.md

[hjohn]

huge pain, yes, that's why I am still working on https://github.com/andy-goryachev/FxDock/blob/master/doc/CPane.md

:-)

I created my own Flex layout, which is a combination of HBox + VBox + GridPane. It basically works by looking at how they are nested. If you nest multiple horizontal flexes into a vertical flex:

 FlexV
       FlexH [ label , text field ]
       FlexH [ label , text field ]

The above would produce an unaligned mess when the labels are different lengths, just like nesting HBoxes into VBox would. However, the Flex has a property "aligned" that you can set, and it will affect the layout of any nested flexes of the opposite type. In this case, if you give the top FlexV this property, the nested horizontal flexes will automatically align as if they were in a grid.

It looks like this in code (in builder form):

    Panes.vflex("form").aligned().nodes(
      Panes.hflex().nodes(
        "Width",
        FX.spinner().editable().value(parameters.width)
      ),
      XPanes.hflex().nodes(
        "Height",
        FX.spinner().editable().value(parameters.height)
      )
    );

I actually only use GridPane in my samples, as I'm not ready yet to publish Flex.