Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8090841

Add a better API for ContextMenu




      This is as far as I understand the problem after a couple of hours trying to make context menus work for TreeTableView, TableView and ListView.

      The current API has a set-in-advance approach. You should set the ContextMenu, which is a Window, on any Control you want it to appear. This is quite wasteful in that context menus are not always used and they are used at most one at a time. It will slow down startup unless extreme precautions are made to add them in a background thread with all the other problems that will give.

      Also, since the ContextMenu is created in advance it is in its nature static. You create the MenuItems in advance and set them. Their content can be made dynamic (target context sensitive) by listening to will-show events, but that is neither encouraged nor a good solution. Especially since according to Tom it needs to have at least one MenuItem to even start the showing.
      In my experience with real-world apps this is seldom the case. One wants to make everything context sensitive to enable, disable or hide (leave out) menu choices that aren't relevant. And sub menus are often built up on demand. Since the ContextMenu property is not writable, and you can't override getContextMenu(), since it's final, there is no natural way to create dynamically created context menus.

      Also, to add ContextMenus to TableCells (and similar) is complex beyond what anyone would expect. There's a solution here, but it is merely a hack and not a good solution for several reasons. Among other that it is not the concern of the Table/List/TreeTableCell to provide the context menu. Neither is it a good idea to proxy a factory since it is a very fragile approach (what happens if you provide means to exchange the cell factory). Context menu creation is something that should be handled at a level or two above the cell.

      So, I propose to add or update the context menu API throughout the JavaFX API with a callback instead. The parameter to the Callback should be the context for the menu. For a most controls it might just be the Control but for TableCells it would be the TableLocation.


      setContextMenu(Callback<TablePosition<S, T>>, ContextMenu> menuProvider) {..}

      for TableView. One might need to make the parameter more generic to account for headers, empty area or other things.

      This would make the ContextMenu both more light-weight (ContextMenus are create as needed) and it would make it MUCH easier to use, especially with lambdas. Literally one-liners in many cases)

      Nothing needs to be deprecated to add this API. The callback should just be, in JavaDoc, mandated to use the set current ContextMenu, if set, just like it is in many paces mandated that the super method is called.

      I hope this can be fixed sooner rather than later. IMO it's a major API flaw that needs to be fixed. No normal developers will create their own proxy-ing cell factory just to add a context menu.




            aghaisas Ajit Ghaisas
            mgrev Mikael Grev
            1 Vote for this issue
            3 Start watching this issue