RFR: 8307960: Create Table Column PopupMenu lazily
Lukasz Kostyra
lkostyra at openjdk.org
Fri May 12 11:16:52 UTC 2023
On Thu, 11 May 2023 19:39:36 GMT, Marius Hanl <mhanl at openjdk.org> 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
More information about the openjfx-dev
mailing list