From 57d7d258f16bf71cc0c4df9f0c8b4984c406c25b Mon Sep 17 00:00:00 2001
From: Simon Barner <barner@fortiss.org>
Date: Thu, 5 Oct 2023 14:38:13 +0200
Subject: [PATCH] TreeContextMenuItem.createTreeContextMenu(): null indicates
 separator

Issue-Ref: 4322
Issue-URL: https://git.fortiss.org/af3/af3/-/issues/4322

Signed-off-by: Simon Barner <barner@fortiss.org>
---
 .../ui/javafx/control/treetableview/.ratings  |  4 +-
 .../DynamicTreeUIProviderBase.java            |  3 +-
 .../treetableview/TreeContextMenuItem.java    | 60 ++++++++++++-------
 3 files changed, 44 insertions(+), 23 deletions(-)

diff --git a/org.fortiss.tooling.common.ui/src/org/fortiss/tooling/common/ui/javafx/control/treetableview/.ratings b/org.fortiss.tooling.common.ui/src/org/fortiss/tooling/common/ui/javafx/control/treetableview/.ratings
index d5d70a079..006a4c29d 100644
--- a/org.fortiss.tooling.common.ui/src/org/fortiss/tooling/common/ui/javafx/control/treetableview/.ratings
+++ b/org.fortiss.tooling.common.ui/src/org/fortiss/tooling/common/ui/javafx/control/treetableview/.ratings
@@ -7,10 +7,10 @@ DynamicTreeItem.java 7e81ea98038b5eca90df583e0268d4e8f37aaf25 GREEN
 DynamicTreeItemBase.java d883066ecc181120302ca32f328538de7a45b093 GREEN
 DynamicTreeTableUIProviderBase.java c52a1f9598de25874f83c133a8cbbcddc86442e9 GREEN
 DynamicTreeTableViewer.java 6e1fcc7a45076d741b80c3a5642a5c688fc651a6 YELLOW
-DynamicTreeUIProviderBase.java 6c186ade3aba51037303ed7a4ad812399bd70c56 YELLOW
+DynamicTreeUIProviderBase.java ba812eca79ef5b0b3962ef440c953dea1a3dcef7 YELLOW
 DynamicTreeViewer.java b0d8cc4b3e11aa970446af12d1e54c750713b297 YELLOW
 DynamicTreeViewerBase.java a2013538b62d86f6a09efdf2cd78babac2072484 GREEN
 EmptyChildrenContentProvider.java 51b4468f9df8423abeea5ac6aa2f6cf99c2eb512 GREEN
 IDoubleClickHandler.java 447f7769dead9a106b3ea3139ef0da51eb0b9a89 GREEN
 IDynamicItem.java 083d02459e7ec33542d9910c04abe2581e0b5422 GREEN
-TreeContextMenuItem.java c7f72dc795c7960f9088126361ead456fa7c02db YELLOW
+TreeContextMenuItem.java e32ff2ee9269e1a7e944bf3b614cdb2ca890d2b5 YELLOW
diff --git a/org.fortiss.tooling.common.ui/src/org/fortiss/tooling/common/ui/javafx/control/treetableview/DynamicTreeUIProviderBase.java b/org.fortiss.tooling.common.ui/src/org/fortiss/tooling/common/ui/javafx/control/treetableview/DynamicTreeUIProviderBase.java
index 6c186ade3..ba812eca7 100644
--- a/org.fortiss.tooling.common.ui/src/org/fortiss/tooling/common/ui/javafx/control/treetableview/DynamicTreeUIProviderBase.java
+++ b/org.fortiss.tooling.common.ui/src/org/fortiss/tooling/common/ui/javafx/control/treetableview/DynamicTreeUIProviderBase.java
@@ -65,7 +65,8 @@ public abstract class DynamicTreeUIProviderBase<T> {
 
 	/**
 	 * Method to define context {@link TreeContextMenuItem}s. Depending on the selected content, the
-	 * editor will show and enable the context menu.
+	 * editor will show and enable the context menu. {@code null} may be used to denote menu
+	 * separators.
 	 */
 	protected void addContextMenuEntry(Class<? extends TreeContextMenuItem<T>> menuItemType) {
 		contextMenuEntryTypes.add(menuItemType);
diff --git a/org.fortiss.tooling.common.ui/src/org/fortiss/tooling/common/ui/javafx/control/treetableview/TreeContextMenuItem.java b/org.fortiss.tooling.common.ui/src/org/fortiss/tooling/common/ui/javafx/control/treetableview/TreeContextMenuItem.java
index c7f72dc79..e32ff2ee9 100644
--- a/org.fortiss.tooling.common.ui/src/org/fortiss/tooling/common/ui/javafx/control/treetableview/TreeContextMenuItem.java
+++ b/org.fortiss.tooling.common.ui/src/org/fortiss/tooling/common/ui/javafx/control/treetableview/TreeContextMenuItem.java
@@ -23,6 +23,7 @@ import javafx.event.EventHandler;
 import javafx.scene.Node;
 import javafx.scene.control.ContextMenu;
 import javafx.scene.control.MenuItem;
+import javafx.scene.control.SeparatorMenuItem;
 
 /**
  * Base class for {@link MenuItem}s used by {@link DynamicTreeViewer}s (but in principle generally
@@ -74,33 +75,52 @@ public abstract class TreeContextMenuItem<T> extends MenuItem {
 	 */
 	protected abstract EventHandler<ActionEvent> createOnActionHandler();
 
-	/** Factory method for a {@link ContextMenu} defined based on {@link TreeContextMenuItem}s. */
+	/**
+	 * Factory method for a {@link ContextMenu} defined based on {@link TreeContextMenuItem}s.
+	 * {@code null} may be used to denote menu separators.
+	 */
 	public static <T> ContextMenu createTreeContextMenu(
 			Collection<Class<? extends TreeContextMenuItem<T>>> contextMenuEntryTypes, T element) {
 
+		boolean previousWasItem = false;
+		boolean needSeparator = false;
 		ContextMenu menu = new ContextMenu();
-		for(Class<? extends TreeContextMenuItem<T>> entryType : contextMenuEntryTypes) {
 
-			try {
-				Constructor<?>[] ctrs = entryType.getConstructors();
-				if(ctrs.length != 1) {
-					throw new RuntimeException(
-							"Subclasses of TreeContextMenuItem must provide exactly one public constructor");
-				}
-				@SuppressWarnings("unchecked") Constructor<? extends TreeContextMenuItem<T>> ctr =
-						(Constructor<? extends TreeContextMenuItem<T>>)ctrs[0];
-				if(ctr.getParameterCount() != 1) {
-					throw new RuntimeException(
-							"The constructor of subclasses of TreeContextMenuItem must take the tree element for which the context menu is created as single argument.");
+		for(Class<? extends TreeContextMenuItem<T>> entryType : contextMenuEntryTypes) {
+			if(entryType == null) {
+				if(previousWasItem) {
+					// Separator is only needed if there is a preceding visible item
+					needSeparator = true;
+					previousWasItem = false;
 				}
-
-				TreeContextMenuItem<?> entry = ctr.newInstance(element);
-				if(!entry.isHidden()) {
-					menu.getItems().add(entry);
-					entry.setDisable(entry.isDisabled());
+			} else {
+				try {
+					Constructor<?>[] ctrs = entryType.getConstructors();
+					if(ctrs.length != 1) {
+						throw new RuntimeException(
+								"Subclasses of TreeContextMenuItem must provide exactly one public constructor");
+					}
+					@SuppressWarnings("unchecked") Constructor<? extends TreeContextMenuItem<T>> ctr =
+							(Constructor<? extends TreeContextMenuItem<T>>)ctrs[0];
+					if(ctr.getParameterCount() != 1) {
+						throw new RuntimeException(
+								"The constructor of subclasses of TreeContextMenuItem must take the tree element for which the context menu is created as single argument.");
+					}
+
+					TreeContextMenuItem<?> entry = ctr.newInstance(element);
+					if(!entry.isHidden()) {
+						// Add separator only if there is a subsequent visible item
+						if(needSeparator) {
+							menu.getItems().add(new SeparatorMenuItem());
+							needSeparator = false;
+						}
+						menu.getItems().add(entry);
+						entry.setDisable(entry.isDisabled());
+						previousWasItem = true;
+					}
+				} catch(Exception e) {
+					e.printStackTrace();
 				}
-			} catch(Exception e) {
-				e.printStackTrace();
 			}
 		}
 
-- 
GitLab