Do not use edit mode to toggle checkboxes in the filter tree

Edit mode has assumptions about the selection mode which are hard to
overcome, and, potentially, may cause other issues. This CL changes
toggle handling to a solution that just tracks clicks on checkboxes.

There is an extra layout to determine where checkbox is, but it should
be fast and only happen for the filter being toggled - not that editor
solution is free from this as well.

Another crucial piece is to use mutable tree nodes in the tree model.
The selection handling keeps track of some TreePath instances and
doesn't update them when the underlying model changes. Immutable filter
instances were part of these TreePath, so enabling/disabling the filter
(which replaces the immutable instance in the model) rendered these
paths invalid, breaking selection tracking.

Issue: fixes #384, fixes #385, fixes #386

src/name/mlopatkin/andlogview/MainFrame.java

@@ -57,6 +57,7 @@
 import name.mlopatkin.andlogview.ui.search.SearchPresenter;
 import name.mlopatkin.andlogview.ui.search.logtable.TablePosition;
 import name.mlopatkin.andlogview.ui.status.StatusPanel;
+import name.mlopatkin.andlogview.ui.themes.Theme;
 import name.mlopatkin.andlogview.utils.CommonChars;
 import name.mlopatkin.andlogview.utils.MyFutures;
 import name.mlopatkin.andlogview.widgets.UiHelper;
@@ -116,6 +117,8 @@ public class MainFrame implements MainFrameSearchUi, DeviceDisconnectedHandler.D
 
     @Inject
     Features features;
+    @Inject
+    Theme theme;
 
     @Inject
     MainFrameUi mainFrameUi;
@@ -342,7 +345,7 @@ private void initLogTable() {
 
         JScrollPane logTableScrollPane = new JScrollPane(logElements);
 
-        if (!features.useFilterTree.isEnabled()) {
+        if (!theme.supportsFilterTreeView() || !features.useFilterTree.isEnabled()) {
             mainFrameUi.getContentPane().add(logTableScrollPane, BorderLayout.CENTER);
             return;
         }

src/name/mlopatkin/andlogview/ui/filterdialog/FilterFromDialogImpl.java

@@ -21,6 +21,8 @@
 import name.mlopatkin.andlogview.logmodel.LogRecord;
 import name.mlopatkin.andlogview.search.RequestCompilationException;
 
+import com.google.common.base.MoreObjects;
+
 import org.checkerframework.checker.nullness.qual.Nullable;
 
 import java.awt.Color;
@@ -86,4 +88,9 @@ public boolean equals(Object obj) {
         }
         return false;
     }
+
+    @Override
+    public String toString() {
+        return MoreObjects.toStringHelper(this).add("enabled", isEnabled()).add("data", getData()).toString();
+    }
 }

src/name/mlopatkin/andlogview/ui/filters/TreeNodeFilter.java

@@ -47,4 +47,9 @@ public String getText() {
     interface Factory {
         TreeNodeFilter create(FilterFromDialog filter);
     }
+
+    @Override
+    public String toString() {
+        return filter.toString();
+    }
 }

src/name/mlopatkin/andlogview/ui/filtertree/FilterNodeRenderer.java

@@ -16,24 +16,29 @@
 
 package name.mlopatkin.andlogview.ui.filtertree;
 
+import name.mlopatkin.andlogview.widgets.checktree.Checkable;
+
 import java.awt.Color;
 import java.awt.Component;
-import java.awt.event.ItemListener;
+import java.awt.Point;
 
 import javax.inject.Inject;
 import javax.swing.BoxLayout;
 import javax.swing.JCheckBox;
+import javax.swing.JComponent;
 import javax.swing.JLabel;
 import javax.swing.JPanel;
 import javax.swing.JTree;
 import javax.swing.UIManager;
 import javax.swing.tree.DefaultTreeCellRenderer;
 import javax.swing.tree.TreeCellRenderer;
 
-class FilterNodeRenderer implements TreeCellRenderer  {
-    private final JPanel panel = new JPanel();
+class FilterNodeRenderer implements TreeCellRenderer {
+    private final ValueRenderer<FilterNodeViewModel> valueRenderer;
+
     private final JCheckBox checkBox = new JCheckBox();
     private final JLabel text = new JLabel();
+    private final CheckablePanel panel;
     private final TreeCellRenderer defaultRenderer = new DefaultTreeCellRenderer();
 
     private final Color selectionForeground;
@@ -45,6 +50,8 @@ class FilterNodeRenderer implements TreeCellRenderer  {
 
     @Inject
     public FilterNodeRenderer() {
+        this.valueRenderer = new ValueRenderer<>(FilterNodeViewModel.class);
+        panel = new CheckablePanel(checkBox);
         panel.setLayout(new BoxLayout(panel, BoxLayout.X_AXIS));
         panel.add(checkBox);
         panel.add(text);
@@ -68,7 +75,8 @@ public FilterNodeRenderer() {
     @Override
     public Component getTreeCellRendererComponent(JTree tree, Object value, boolean selected, boolean expanded,
             boolean leaf, int row, boolean hasFocus) {
-        if (!(value instanceof FilterNodeViewModel filterNode)) {
+        var filter = valueRenderer.toModel(value);
+        if (filter == null) {
             // Sometimes we're asked for the root node even though it is invisible. Fall back to the default renderer in
             // this case.
             return defaultRenderer.getTreeCellRendererComponent(tree, value, selected, expanded, leaf, row, hasFocus);
@@ -82,8 +90,8 @@ public Component getTreeCellRendererComponent(JTree tree, Object value, boolean
         panel.setBackground(bgColor);
         text.setBackground(bgColor);
 
-        checkBox.setSelected(filterNode.isEnabled());
-        text.setText(filterNode.getText());
+        checkBox.setSelected(filter.isEnabled());
+        text.setText(filter.getText());
 
         if (!checkBox.isValid() || !text.isValid()) {
             // When the panel is detached, it doesn't invalidate itself upon child invalidation. This causes stale
@@ -117,7 +125,16 @@ public boolean isSelected() {
         return checkBox.isSelected();
     }
 
-    public void addCheckboxItemListener(ItemListener listener) {
-        checkBox.addItemListener(listener);
+    private static class CheckablePanel extends JPanel implements Checkable {
+        private final JComponent checkTarget;
+
+        public CheckablePanel(JComponent checkTarget) {
+            this.checkTarget = checkTarget;
+        }
+
+        @Override
+        public boolean togglesCheckable(Point mouseClickInNode) {
+            return checkTarget.getBounds().contains(mouseClickInNode);
+        }
     }
 }

src/name/mlopatkin/andlogview/ui/filtertree/FilterTreeFactory.java

@@ -16,11 +16,9 @@
 
 package name.mlopatkin.andlogview.ui.filtertree;
 
-import java.awt.event.FocusAdapter;
-import java.awt.event.FocusEvent;
+import name.mlopatkin.andlogview.widgets.checktree.CheckFlatTreeUi;
 
 import javax.inject.Inject;
-import javax.inject.Provider;
 import javax.swing.JTree;
 import javax.swing.tree.DefaultTreeSelectionModel;
 
@@ -30,44 +28,25 @@
 public class FilterTreeFactory {
     private final FilterNodeRenderer nodeRenderer;
     private final TreeModelAdapter treeModel;
-    private final Provider<FilterNodeEditor> editorProvider;
 
     @Inject
     FilterTreeFactory(
             FilterNodeRenderer nodeRenderer,
-            TreeModelAdapter treeModel,
-            Provider<FilterNodeEditor> editorProvider) {
+            TreeModelAdapter treeModel) {
         this.nodeRenderer = nodeRenderer;
         this.treeModel = treeModel;
-        this.editorProvider = editorProvider;
     }
 
     public JTree buildFilterTree() {
         var filterTree = new JTree(treeModel);
 
+        filterTree.setUI(new CheckFlatTreeUi());
+
         filterTree.setRootVisible(false);
         filterTree.setShowsRootHandles(false);
         filterTree.setCellRenderer(nodeRenderer);
         filterTree.setSelectionModel(new DefaultTreeSelectionModel());
 
-        filterTree.setEditable(true);
-        filterTree.setCellEditor(editorProvider.get());
-        filterTree.addFocusListener(new FocusAdapter() {
-            @Override
-            public void focusLost(FocusEvent e) {
-                // The filter editing is basically providing an active checkbox to toggle. This checkbox doesn't reflect
-                // updates to model made from the outside (e.g. disabling the index window filter by closing its
-                // window). To make sure we always show the actual filter state, we actually discard the editing session
-                // if the user goes elsewhere.
-                // TODO(mlopatkin): The editor keeps a reference to the filter being edited. If the underlying model
-                //  changes, then the filter may no longer be in the model. Should we protect ourselves from
-                //  accidentally modifying the outdated filter?
-                if (filterTree.isEditing()) {
-                    filterTree.cancelEditing();
-                }
-            }
-        });
-
         return filterTree;
     }
 }