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