On Thu, 11 May 2023 19:39:36 GMT, Marius Hanl <[email protected]> wrote:
> This PR changes the `columnPopupMenu`, so that it is created lazily.
>
> The problem here is, that the `columnPopupMenu` is always initialized and
> updated via bindings, even if the table menu button is never shown
> (`setTableMenuButtonVisible(false)`) or the user never clicked on it.
> This problem can be solved by creating the `columnPopupMenu` and related
> bindings when it should be shown the first time.
>
> I also added many tests to ensure that everything still works (there are no
> tests for that area as of now).
>
> Side note: There are a bunch of tickets with the wish to customize the Popup
> shown by the table menu button or show it programmatically. This ticket will
> prepare this, as now all Popup related code is on one place and in the future
> we can think of implementing a way to override this behaviour in a way that
> the Popup and all related bindings are never created and therefore do not
> decrease performance.
Left a couple very minor style-related comments
modules/javafx.controls/src/shims/java/javafx/scene/control/skin/TableHeaderRowShim.java
line 38:
> 36:
> 37: public static ContextMenu getColumnPopupMenu(TableHeaderRow
> tableHeaderRow) {
> 38: return tableHeaderRow.getColumnPopupMenu();
Minor: I would keep `tr` instead of `tableHeaderRow` for consistency with
already existing static method.
modules/javafx.controls/src/shims/java/javafx/scene/control/skin/TableHeaderRowShim.java
line 42:
> 40:
> 41: public static Pane getCornerRegion(TableHeaderRow tableHeaderRow) {
> 42: return tableHeaderRow.getCornerRegion();
As above
-------------
PR Review: https://git.openjdk.org/jfx/pull/1133#pullrequestreview-1424274489
PR Review Comment: https://git.openjdk.org/jfx/pull/1133#discussion_r1192230921
PR Review Comment: https://git.openjdk.org/jfx/pull/1133#discussion_r1192230993