On Mon, 15 May 2023 22:39:52 GMT, Marius Hanl <mh...@openjdk.org> wrote:

>> This PR implements a way to override the table column menu. 
>> When the `cornerRegion` is pressed, it will now call the `showColumnMenu` 
>> method. This new method is protected and therefore can be overidden by 
>> developers. If not overridden, the known default column 'ContextMenu' is 
>> created and shown.
>> 
>> This PR also resolves [JDK-8091419 
>> ](https://bugs.openjdk.org/browse/JDK-8091419) (The method `showColumnMenu` 
>> can be overridden and made public now)
>> 
>> This PR also helps with 
>> [JDK-8092148](https://bugs.openjdk.org/browse/JDK-8092148), but does not 
>> address all the points mentioned in the ticket. 
>> Now that everyone can provide their own implementation, we can think of 
>> treating the current implementation of the table column menu as an 
>> [MVP](https://en.wikipedia.org/wiki/Minimum_viable_product) and any other 
>> requested feature/idea should be fulfilled by providing your own 
>> implementaion of the menu.
>
> Marius Hanl has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   JDK-8091153: Improve javadoc to match the CSR

The new API looks OK to me. I left a couple comments on the API docs.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableHeaderRow.java
 line 468:

> 466: 
> 467:     /**
> 468:      * Shows a menu containing all leaf columns as item.

"item" --> "items"

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableHeaderRow.java
 line 472:

> 470:      *
> 471:      * @implNote This method can be overridden to create and show a 
> custom menu.
> 472:      * @param mouseEvent the {@link MouseEvent} which was generated when 
> the table menu button was pressed.

Minor: by convention, the text following `@param` and `@return` tags should not 
end with a period.

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

PR Review: https://git.openjdk.org/jfx/pull/1135#pullrequestreview-1430646782
PR Review Comment: https://git.openjdk.org/jfx/pull/1135#discussion_r1196490418
PR Review Comment: https://git.openjdk.org/jfx/pull/1135#discussion_r1196493786

Reply via email to