Bug 528857 - [Tree] Expand/collapse not working with spanned tree cells

Change-Id: I4750cd2f68a525a7b7da1968d210b98a3854ec4d
Signed-off-by: Dirk Fauth <dirk.fauth@googlemail.com>

3 files changed, 87 insertions, 34 deletions

diff --git a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/tree/TreeLayer.java b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/tree/TreeLayer.java
index 89f26e8b..8a05bb59 100644
--- a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/tree/TreeLayer.java
+++ b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/tree/TreeLayer.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2012, 2016 Original authors and others.
+ * Copyright (c) 2012, 2017 Original authors and others.
  * 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
@@ -43,7 +43,7 @@ import org.eclipse.nebula.widgets.nattable.tree.painter.IndentedTreeImagePainter
 
 public class TreeLayer extends AbstractRowHideShowLayer {
 
-    private static final Log log = LogFactory.getLog(TreeLayer.class);
+    private static final Log LOG = LogFactory.getLog(TreeLayer.class);
 
     public static final String TREE_COLUMN_CELL = "TREE_COLUMN_CELL"; //$NON-NLS-1$
 
@@ -190,16 +190,19 @@ public class TreeLayer extends AbstractRowHideShowLayer {
         if (isTreeColumn(columnPosition)) {
             configLabels.addLabelOnTop(TREE_COLUMN_CELL);
 
-            int rowIndex = getRowIndexByPosition(rowPosition);
-            configLabels.addLabelOnTop(
-                    DefaultTreeLayerConfiguration.TREE_DEPTH_CONFIG_TYPE + this.treeRowModel.depth(rowIndex));
-            if (!this.treeRowModel.hasChildren(rowIndex)) {
-                configLabels.addLabelOnTop(DefaultTreeLayerConfiguration.TREE_LEAF_CONFIG_TYPE);
-            } else {
-                if (this.treeRowModel.isCollapsed(rowIndex)) {
-                    configLabels.addLabelOnTop(DefaultTreeLayerConfiguration.TREE_COLLAPSED_CONFIG_TYPE);
+            ILayerCell cell = getCellByPosition(columnPosition, rowPosition);
+            if (cell != null) {
+                int rowIndex = cell.getOriginRowPosition();
+                configLabels.addLabelOnTop(
+                        DefaultTreeLayerConfiguration.TREE_DEPTH_CONFIG_TYPE + this.treeRowModel.depth(rowIndex));
+                if (!this.treeRowModel.hasChildren(rowIndex)) {
+                    configLabels.addLabelOnTop(DefaultTreeLayerConfiguration.TREE_LEAF_CONFIG_TYPE);
                 } else {
-                    configLabels.addLabelOnTop(DefaultTreeLayerConfiguration.TREE_EXPANDED_CONFIG_TYPE);
+                    if (this.treeRowModel.isCollapsed(rowIndex)) {
+                        configLabels.addLabelOnTop(DefaultTreeLayerConfiguration.TREE_COLLAPSED_CONFIG_TYPE);
+                    } else {
+                        configLabels.addLabelOnTop(DefaultTreeLayerConfiguration.TREE_EXPANDED_CONFIG_TYPE);
+                    }
                 }
             }
         }
@@ -249,8 +252,9 @@ public class TreeLayer extends AbstractRowHideShowLayer {
      *            The column position to check.
      * @return <code>true</code> if the given column position is the tree
      *         column, <code>false</code> if not.
+     * @since 1.6
      */
-    private boolean isTreeColumn(int columnPosition) {
+    protected boolean isTreeColumn(int columnPosition) {
         if (this.useTreeColumnIndex)
             return getColumnIndexByPosition(columnPosition) == TREE_COLUMN_NUMBER;
 
@@ -279,7 +283,7 @@ public class TreeLayer extends AbstractRowHideShowLayer {
                     cellPainter = treeCellPainter;
                 } else {
                     // log error
-                    log.warn("There is no IndentedTreeImagePainter found for TREE_STRUCTURE_PAINTER, " //$NON-NLS-1$
+                    LOG.warn("There is no IndentedTreeImagePainter found for TREE_STRUCTURE_PAINTER, " //$NON-NLS-1$
                             + "using local configured IndentedTreeImagePainter as fallback"); //$NON-NLS-1$
                     // fallback
                     this.indentedTreeImagePainter.setBaseCellPainter(cellPainter);
diff --git a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/tree/action/TreeExpandCollapseAction.java b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/tree/action/TreeExpandCollapseAction.java
index 7b6b13b4..fdaa3cdb 100644
--- a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/tree/action/TreeExpandCollapseAction.java
+++ b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/tree/action/TreeExpandCollapseAction.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2012 Original authors and others.
+ * Copyright (c) 2012, 2017 Original authors and others.
  * 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
@@ -13,9 +13,17 @@ package org.eclipse.nebula.widgets.nattable.tree.action;
 import org.eclipse.nebula.widgets.nattable.NatTable;
 import org.eclipse.nebula.widgets.nattable.layer.cell.ILayerCell;
 import org.eclipse.nebula.widgets.nattable.tree.command.TreeExpandCollapseCommand;
+import org.eclipse.nebula.widgets.nattable.tree.config.DefaultTreeLayerConfiguration;
 import org.eclipse.nebula.widgets.nattable.ui.action.IMouseAction;
 import org.eclipse.swt.events.MouseEvent;
 
+/**
+ * {@link IMouseAction} that triggers a {@link TreeExpandCollapseCommand} for
+ * the clicked cell position. By default registered on click on the tree
+ * expand/collapse icon.
+ *
+ * @see DefaultTreeLayerConfiguration
+ */
 public class TreeExpandCollapseAction implements IMouseAction {
 
     @Override
@@ -23,8 +31,7 @@ public class TreeExpandCollapseAction implements IMouseAction {
         int c = natTable.getColumnPositionByX(event.x);
         int r = natTable.getRowPositionByY(event.y);
         ILayerCell cell = natTable.getCellByPosition(c, r);
-        int index = cell.getLayer()
-                .getRowIndexByPosition(cell.getRowPosition());
+        int index = cell.getLayer().getRowIndexByPosition(cell.getOriginRowPosition());
         TreeExpandCollapseCommand command = new TreeExpandCollapseCommand(index);
         natTable.doCommand(command);
     }
diff --git a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/ui/matcher/CellPainterMouseEventMatcher.java b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/ui/matcher/CellPainterMouseEventMatcher.java
index 8b682cde..64941e68 100644
--- a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/ui/matcher/CellPainterMouseEventMatcher.java
+++ b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/ui/matcher/CellPainterMouseEventMatcher.java
@@ -12,6 +12,7 @@ package org.eclipse.nebula.widgets.nattable.ui.matcher;
 
 import org.eclipse.nebula.widgets.nattable.NatTable;
 import org.eclipse.nebula.widgets.nattable.config.IConfigRegistry;
+import org.eclipse.nebula.widgets.nattable.grid.GridRegion;
 import org.eclipse.nebula.widgets.nattable.layer.LabelStack;
 import org.eclipse.nebula.widgets.nattable.layer.cell.ILayerCell;
 import org.eclipse.nebula.widgets.nattable.painter.cell.ICellPainter;
@@ -20,34 +21,68 @@ import org.eclipse.swt.graphics.GC;
 import org.eclipse.swt.graphics.Rectangle;
 
 /**
- * Matches a mouse click on a given cell painter within a cell.
+ * Matches a mouse click on an {@link ICellPainter} within a cell in a specified
+ * region.
  */
 public class CellPainterMouseEventMatcher extends MouseEventMatcher {
 
     private ICellPainter targetCellPainter;
     private Class<? extends ICellPainter> targetCellPainterClass;
 
-    public CellPainterMouseEventMatcher(String regionName, int button,
-            ICellPainter targetCellPainter) {
+    /**
+     * Creates a {@link CellPainterMouseEventMatcher} for a given region name,
+     * mouse button and a specific target cellPainter instance. Can be used in
+     * case one single painter instance is registered that should be checked
+     * for.
+     *
+     * @param regionName
+     *            The name of the region where this matcher should react on.
+     *            Typically a {@link GridRegion} if a default grid is used. Can
+     *            also be another value in case a custom region label is defined
+     *            in a custom composition.
+     * @param button
+     *            The mouse button this matcher should react on, e.g.
+     *            {@link MouseEventMatcher#LEFT_BUTTON} and
+     *            {@link MouseEventMatcher#RIGHT_BUTTON}
+     * @param targetCellPainter
+     *            The {@link ICellPainter} instance that should be used for the
+     *            check.
+     */
+    public CellPainterMouseEventMatcher(String regionName, int button, ICellPainter targetCellPainter) {
         super(regionName, button);
         this.targetCellPainter = targetCellPainter;
     }
 
-    public CellPainterMouseEventMatcher(String regionName, int button,
-            Class<? extends ICellPainter> targetCellPainterClass) {
+    /**
+     * Creates a {@link CellPainterMouseEventMatcher} for a given region name,
+     * mouse button and a target cellPainter class. Can be used in case every
+     * instance of a painter should be treated the same way, e.g. checkboxes.
+     *
+     * @param regionName
+     *            The name of the region where this matcher should react on.
+     *            Typically a {@link GridRegion} if a default grid is used. Can
+     *            also be another value in case a custom region label is defined
+     *            in a custom composition.
+     * @param button
+     *            The mouse button this matcher should react on, e.g.
+     *            {@link MouseEventMatcher#LEFT_BUTTON} and
+     *            {@link MouseEventMatcher#RIGHT_BUTTON}
+     * @param targetCellPainterClass
+     *            The concrete type of the {@link ICellPainter} that should be
+     *            used for the check.
+     */
+    public CellPainterMouseEventMatcher(String regionName, int button, Class<? extends ICellPainter> targetCellPainterClass) {
         super(regionName, button);
         this.targetCellPainterClass = targetCellPainterClass;
     }
 
     @Override
-    public boolean matches(NatTable natTable, MouseEvent event,
-            LabelStack regionLabels) {
+    public boolean matches(NatTable natTable, MouseEvent event, LabelStack regionLabels) {
         if (super.matches(natTable, event, regionLabels)) {
             int columnPosition = natTable.getColumnPositionByX(event.x);
             int rowPosition = natTable.getRowPositionByY(event.y);
 
-            ILayerCell cell = natTable.getCellByPosition(columnPosition,
-                    rowPosition);
+            ILayerCell cell = natTable.getCellByPosition(columnPosition, rowPosition);
 
             // Bug 407598: only perform a check if the click in the body region
             // was performed on a cell
@@ -56,21 +91,28 @@ public class CellPainterMouseEventMatcher extends MouseEventMatcher {
             if (cell != null) {
                 IConfigRegistry configRegistry = natTable.getConfigRegistry();
                 ICellPainter cellPainter = cell.getLayer().getCellPainter(
-                        columnPosition, rowPosition, cell, configRegistry);
+                        columnPosition,
+                        rowPosition,
+                        cell,
+                        configRegistry);
 
                 GC gc = new GC(natTable.getDisplay());
                 try {
-                    Rectangle adjustedCellBounds = natTable.getLayerPainter()
-                            .adjustCellBounds(columnPosition, rowPosition,
-                                    cell.getBounds());
+                    Rectangle adjustedCellBounds = natTable.getLayerPainter().adjustCellBounds(
+                            columnPosition,
+                            rowPosition,
+                            cell.getBounds());
 
-                    ICellPainter clickedCellPainter = cellPainter
-                            .getCellPainterAt(event.x, event.y, cell, gc,
-                                    adjustedCellBounds, configRegistry);
+                    ICellPainter clickedCellPainter = cellPainter.getCellPainterAt(
+                            event.x,
+                            event.y,
+                            cell,
+                            gc,
+                            adjustedCellBounds,
+                            configRegistry);
                     if (clickedCellPainter != null) {
                         if ((this.targetCellPainter != null && this.targetCellPainter == clickedCellPainter)
-                                || (this.targetCellPainterClass != null && this.targetCellPainterClass
-                                        .isInstance(clickedCellPainter))) {
+                                || (this.targetCellPainterClass != null && this.targetCellPainterClass.isInstance(clickedCellPainter))) {
                             return true;
                         }
                     }