Alarm tree: Fix vanishing open/close tirangle and 'cursor'

#2608

app/alarm/ui/src/main/java/org/phoebus/applications/alarm/ui/tree/AlarmTreeViewCell.java

@@ -1,42 +1,51 @@
 /*******************************************************************************
- * Copyright (c) 2018 Oak Ridge National Laboratory.
+ * Copyright (c) 2018-2023 Oak Ridge National Laboratory.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
  * http://www.eclipse.org/legal/epl-v10.html
  *******************************************************************************/
 package org.phoebus.applications.alarm.ui.tree;
 
+import javafx.scene.control.Label;
+import javafx.scene.control.TreeCell;
+import javafx.scene.image.ImageView;
 import javafx.scene.layout.Background;
+import javafx.scene.layout.HBox;
+import javafx.scene.paint.Color;
+
 import org.phoebus.applications.alarm.client.AlarmClientLeaf;
 import org.phoebus.applications.alarm.client.AlarmClientNode;
 import org.phoebus.applications.alarm.client.ClientState;
 import org.phoebus.applications.alarm.model.AlarmTreeItem;
 import org.phoebus.applications.alarm.model.SeverityLevel;
 import org.phoebus.applications.alarm.ui.AlarmUI;
 
-import javafx.scene.control.TreeCell;
-import javafx.scene.image.Image;
-import javafx.scene.image.ImageView;
-import javafx.scene.paint.Color;
-
 /** TreeCell for AlarmTreeItem
  *  @author Kay Kasemir
  */
 @SuppressWarnings("nls")
 class AlarmTreeViewCell extends TreeCell<AlarmTreeItem<?>>
 {
+    // Originally, the tree cell "graphics" were used for the icon,
+    // and the built-in label/text that can be controlled via
+    // setText and setBackground for the text.
+    // But using that built-in label/text background intermittently removes
+    // the "triangle" for expanding/collapsing subtrees
+    // as well as the "cursor" for selecting tree cells or navigating
+    // cells via cursor keys.
+    // So we add our own "graphics" to hold an icon and text
+    private final Label label = new Label();
+    private final ImageView image = new ImageView();
+    private final HBox content = new HBox(image, label);
+
     @Override
     protected void updateItem(final AlarmTreeItem<?> item, final boolean empty)
     {
         super.updateItem(item, empty);
-        // Note: Cannot use background because that's used by style sheet and selection/cursor
+
         if (empty  ||  item == null)
-        {
-            setText(null);
             setGraphic(null);
-            setBackground(Background.EMPTY);
-        }
         else
         {
             final SeverityLevel severity;
@@ -46,7 +55,6 @@ protected void updateItem(final AlarmTreeItem<?> item, final boolean empty)
                 final ClientState state = leaf.getState();
 
                 final StringBuilder text = new StringBuilder();
-                final Image icon;
                 text.append("PV: ").append(leaf.getName());
 
                 if (leaf.isEnabled()  &&  !state.isDynamicallyDisabled())
@@ -61,31 +69,32 @@ protected void updateItem(final AlarmTreeItem<?> item, final boolean empty)
                                 .append(state.current_severity).append('/').append(state.current_message)
                                 .append(")");
                     }
-                    setTextFill(AlarmUI.getColor(state.severity));
-                    setBackground(AlarmUI.getBackground(state.severity));
-                    icon = AlarmUI.getIcon(state.severity);
+                    label.setTextFill(AlarmUI.getColor(state.severity));
+                    label.setBackground(AlarmUI.getBackground(state.severity));
+                    image.setImage(AlarmUI.getIcon(state.severity));
                 }
                 else
                 {
                     text.append(" (disabled)");
-                    setTextFill(Color.GRAY);
-                    setBackground(Background.EMPTY);
-                    icon = AlarmUI.disabled_icon;
+                    label.setTextFill(Color.GRAY);
+                    label.setBackground(Background.EMPTY);
+                    image.setImage(AlarmUI.disabled_icon);
                 }
-                setText(text.toString());
-                setGraphic(icon == null ? null : new ImageView(icon));
+                label.setText(text.toString());
             }
             else
             {
                 final AlarmClientNode node = (AlarmClientNode) item;
-                setText(item.getName());
+                label.setText(item.getName());
 
                 severity = node.getState().severity;
-                setTextFill(AlarmUI.getColor(severity));
-                setBackground(AlarmUI.getBackground(severity));
-                final Image icon = AlarmUI.getIcon(severity);
-                setGraphic(icon == null ? null : new ImageView(icon));
+                label.setTextFill(AlarmUI.getColor(severity));
+                label.setBackground(AlarmUI.getBackground(severity));
+                image.setImage(AlarmUI.getIcon(severity));
             }
+            // Profiler showed small advantage when skipping redundant 'setGraphic' call
+            if (getGraphic() != content)
+                setGraphic(content);
         }
     }
 }