On Fri, 12 May 2023 11:08:57 GMT, Lukasz Kostyra <[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.
>
> 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.
I would too - avoids wrapping the line, and there is no need to spell the name
for a one line method.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1133#discussion_r1192630713
