Bug 580368 - Avoid horizontal scrolling on row selection using keys

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

Change-Id: Ic7ac621b3e846e54aea35b642304b774f67792f1

3 files changed, 330 insertions, 60 deletions

diff --git a/org.eclipse.nebula.widgets.nattable.core.test/src/org/eclipse/nebula/widgets/nattable/selection/MoveRowSelectionCommandHandlerTest.java b/org.eclipse.nebula.widgets.nattable.core.test/src/org/eclipse/nebula/widgets/nattable/selection/MoveRowSelectionCommandHandlerTest.java
new file mode 100644
index 00000000..b871a589
--- /dev/null
+++ b/org.eclipse.nebula.widgets.nattable.core.test/src/org/eclipse/nebula/widgets/nattable/selection/MoveRowSelectionCommandHandlerTest.java
@@ -0,0 +1,247 @@
+/*******************************************************************************
+ * Copyright (c) 2022 Dirk Fauth.
+ *
+ * This program and the accompanying materials are made
+ * available under the terms of the Eclipse Public License 2.0
+ * which is available at https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *     Dirk Fauth <dirk.fauth@googlemail.com> - initial API and implementation
+ ******************************************************************************/
+package org.eclipse.nebula.widgets.nattable.selection;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.io.Serializable;
+import java.util.Arrays;
+
+import org.eclipse.nebula.widgets.nattable.data.IRowDataProvider;
+import org.eclipse.nebula.widgets.nattable.data.IRowIdAccessor;
+import org.eclipse.nebula.widgets.nattable.data.ListDataProvider;
+import org.eclipse.nebula.widgets.nattable.data.ReflectiveColumnPropertyAccessor;
+import org.eclipse.nebula.widgets.nattable.dataset.fixture.data.RowDataFixture;
+import org.eclipse.nebula.widgets.nattable.dataset.fixture.data.RowDataListFixture;
+import org.eclipse.nebula.widgets.nattable.layer.DataLayer;
+import org.eclipse.nebula.widgets.nattable.selection.SelectionLayer.MoveDirectionEnum;
+import org.eclipse.nebula.widgets.nattable.selection.command.MoveSelectionCommand;
+import org.eclipse.nebula.widgets.nattable.selection.command.SelectRowsCommand;
+import org.eclipse.nebula.widgets.nattable.viewport.ViewportLayer;
+import org.eclipse.swt.graphics.Rectangle;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class MoveRowSelectionCommandHandlerTest {
+
+    private SelectionLayer selectionLayer;
+    private ViewportLayer viewportLayer;
+
+    @Before
+    public void setUp() {
+        // only use 10 columns to make the test cases easier
+        String[] propertyNames = Arrays.copyOfRange(RowDataListFixture.getPropertyNames(), 0, 10);
+
+        IRowDataProvider<RowDataFixture> bodyDataProvider = new ListDataProvider<RowDataFixture>(
+                RowDataListFixture.getList(10),
+                new ReflectiveColumnPropertyAccessor<RowDataFixture>(propertyNames));
+
+        this.selectionLayer = new SelectionLayer(new DataLayer(bodyDataProvider));
+        this.selectionLayer.registerCommandHandler(new MoveRowSelectionCommandHandler(this.selectionLayer));
+
+        this.selectionLayer.setSelectionModel(new RowSelectionModel<RowDataFixture>(
+                this.selectionLayer, bodyDataProvider,
+                new IRowIdAccessor<RowDataFixture>() {
+
+                    @Override
+                    public Serializable getRowId(RowDataFixture rowObject) {
+                        return rowObject.getSecurity_id();
+                    }
+
+                }));
+
+        this.viewportLayer = new ViewportLayer(this.selectionLayer);
+        // only show 6 columns
+        this.viewportLayer.setClientAreaProvider(() -> new Rectangle(0, 0, 600, 200));
+    }
+
+    @After
+    public void cleanUp() {
+        this.selectionLayer.clear();
+    }
+
+    @Test
+    public void testMoveDown() {
+        // select a row
+        this.viewportLayer.doCommand(new SelectRowsCommand(this.viewportLayer, 1, 4, false, false));
+        assertEquals(1, this.selectionLayer.getSelectionAnchor().getColumnPosition());
+        assertEquals(4, this.selectionLayer.getSelectionAnchor().getRowPosition());
+        assertTrue(this.selectionLayer.isRowPositionFullySelected(4));
+        assertEquals(0, this.viewportLayer.getColumnIndexByPosition(0));
+
+        // move down
+        this.viewportLayer.doCommand(new MoveSelectionCommand(MoveDirectionEnum.DOWN, false, false));
+
+        // anchor moved one row down
+        assertEquals(1, this.selectionLayer.getSelectionAnchor().getColumnPosition());
+        assertEquals(5, this.selectionLayer.getSelectionAnchor().getRowPosition());
+
+        // row selection has changed
+        assertFalse(this.selectionLayer.isRowPositionFullySelected(4));
+        assertTrue(this.selectionLayer.isRowPositionFullySelected(5));
+
+        // viewport has not scrolled
+        assertEquals(0, this.viewportLayer.getColumnIndexByPosition(0));
+    }
+
+    @Test
+    public void testMoveUp() {
+        // select a row
+        this.viewportLayer.doCommand(new SelectRowsCommand(this.viewportLayer, 1, 4, false, false));
+        assertEquals(1, this.selectionLayer.getSelectionAnchor().getColumnPosition());
+        assertEquals(4, this.selectionLayer.getSelectionAnchor().getRowPosition());
+        assertTrue(this.selectionLayer.isRowPositionFullySelected(4));
+        assertEquals(0, this.viewportLayer.getColumnIndexByPosition(0));
+
+        // move down
+        this.viewportLayer.doCommand(new MoveSelectionCommand(MoveDirectionEnum.UP, false, false));
+
+        // anchor moved one row up
+        assertEquals(1, this.selectionLayer.getSelectionAnchor().getColumnPosition());
+        assertEquals(3, this.selectionLayer.getSelectionAnchor().getRowPosition());
+
+        // row selection has changed
+        assertFalse(this.selectionLayer.isRowPositionFullySelected(4));
+        assertTrue(this.selectionLayer.isRowPositionFullySelected(3));
+
+        // viewport has not scrolled
+        assertEquals(0, this.viewportLayer.getColumnIndexByPosition(0));
+    }
+
+    @Test
+    public void testMoveDownShift() {
+        // select a row
+        this.viewportLayer.doCommand(new SelectRowsCommand(this.viewportLayer, 1, 4, false, false));
+        assertEquals(1, this.selectionLayer.getSelectionAnchor().getColumnPosition());
+        assertEquals(4, this.selectionLayer.getSelectionAnchor().getRowPosition());
+        assertTrue(this.selectionLayer.isRowPositionFullySelected(4));
+        assertEquals(0, this.viewportLayer.getColumnIndexByPosition(0));
+
+        // move down with shift
+        this.viewportLayer.doCommand(new MoveSelectionCommand(MoveDirectionEnum.DOWN, true, false));
+
+        // selection anchor stays
+        assertEquals(1, this.selectionLayer.getSelectionAnchor().getColumnPosition());
+        assertEquals(4, this.selectionLayer.getSelectionAnchor().getRowPosition());
+
+        // both rows are selected
+        assertTrue(this.selectionLayer.isRowPositionFullySelected(4));
+        assertTrue(this.selectionLayer.isRowPositionFullySelected(5));
+
+        // viewport has not scrolled
+        assertEquals(0, this.viewportLayer.getColumnIndexByPosition(0));
+    }
+
+    @Test
+    public void testMoveUpShift() {
+        // select a row
+        this.viewportLayer.doCommand(new SelectRowsCommand(this.viewportLayer, 1, 4, false, false));
+        assertEquals(1, this.selectionLayer.getSelectionAnchor().getColumnPosition());
+        assertEquals(4, this.selectionLayer.getSelectionAnchor().getRowPosition());
+        assertTrue(this.selectionLayer.isRowPositionFullySelected(4));
+        assertEquals(0, this.viewportLayer.getColumnIndexByPosition(0));
+
+        // move up
+        this.viewportLayer.doCommand(new MoveSelectionCommand(MoveDirectionEnum.UP, true, false));
+
+        // selection anchor stays
+        assertEquals(1, this.selectionLayer.getSelectionAnchor().getColumnPosition());
+        assertEquals(4, this.selectionLayer.getSelectionAnchor().getRowPosition());
+
+        // both rows are selected
+        assertTrue(this.selectionLayer.isRowPositionFullySelected(4));
+        assertTrue(this.selectionLayer.isRowPositionFullySelected(3));
+
+        // viewport has not scrolled
+        assertEquals(0, this.viewportLayer.getColumnIndexByPosition(0));
+    }
+
+    @Test
+    public void testMoveDownUpShift() {
+        // select a row
+        this.viewportLayer.doCommand(new SelectRowsCommand(this.viewportLayer, 1, 4, false, false));
+        assertEquals(1, this.selectionLayer.getSelectionAnchor().getColumnPosition());
+        assertEquals(4, this.selectionLayer.getSelectionAnchor().getRowPosition());
+        assertTrue(this.selectionLayer.isRowPositionFullySelected(4));
+        assertEquals(0, this.viewportLayer.getColumnIndexByPosition(0));
+
+        // move down
+        this.viewportLayer.doCommand(new MoveSelectionCommand(MoveDirectionEnum.DOWN, true, false));
+
+        // selection anchor stays
+        assertEquals(1, this.selectionLayer.getSelectionAnchor().getColumnPosition());
+        assertEquals(4, this.selectionLayer.getSelectionAnchor().getRowPosition());
+
+        // both rows are selected
+        assertTrue(this.selectionLayer.isRowPositionFullySelected(4));
+        assertTrue(this.selectionLayer.isRowPositionFullySelected(5));
+
+        // viewport has not scrolled
+        assertEquals(0, this.viewportLayer.getColumnIndexByPosition(0));
+
+        // move up again which should result in deselection
+        this.viewportLayer.doCommand(new MoveSelectionCommand(MoveDirectionEnum.UP, true, false));
+
+        // selection anchor stays
+        assertEquals(1, this.selectionLayer.getSelectionAnchor().getColumnPosition());
+        assertEquals(4, this.selectionLayer.getSelectionAnchor().getRowPosition());
+
+        // only one row selected
+        assertTrue(this.selectionLayer.isRowPositionFullySelected(4));
+        assertFalse(this.selectionLayer.isRowPositionFullySelected(5));
+
+        // viewport has not scrolled
+        assertEquals(0, this.viewportLayer.getColumnIndexByPosition(0));
+    }
+
+    @Test
+    public void testMoveUpDownShift() {
+        // select a row
+        this.viewportLayer.doCommand(new SelectRowsCommand(this.viewportLayer, 1, 4, false, false));
+        assertEquals(1, this.selectionLayer.getSelectionAnchor().getColumnPosition());
+        assertEquals(4, this.selectionLayer.getSelectionAnchor().getRowPosition());
+        assertTrue(this.selectionLayer.isRowPositionFullySelected(4));
+        assertEquals(0, this.viewportLayer.getColumnIndexByPosition(0));
+
+        // move up
+        this.viewportLayer.doCommand(new MoveSelectionCommand(MoveDirectionEnum.UP, true, false));
+
+        // selection anchor stays
+        assertEquals(1, this.selectionLayer.getSelectionAnchor().getColumnPosition());
+        assertEquals(4, this.selectionLayer.getSelectionAnchor().getRowPosition());
+
+        // both rows are selected
+        assertTrue(this.selectionLayer.isRowPositionFullySelected(4));
+        assertTrue(this.selectionLayer.isRowPositionFullySelected(3));
+
+        // viewport has not scrolled
+        assertEquals(0, this.viewportLayer.getColumnIndexByPosition(0));
+
+        // move down again which should result in deselection
+        this.viewportLayer.doCommand(new MoveSelectionCommand(MoveDirectionEnum.DOWN, true, false));
+
+        // selection anchor stays
+        assertEquals(1, this.selectionLayer.getSelectionAnchor().getColumnPosition());
+        assertEquals(4, this.selectionLayer.getSelectionAnchor().getRowPosition());
+
+        // only one row selected
+        assertTrue(this.selectionLayer.isRowPositionFullySelected(4));
+        assertFalse(this.selectionLayer.isRowPositionFullySelected(3));
+
+        // viewport has not scrolled
+        assertEquals(0, this.viewportLayer.getColumnIndexByPosition(0));
+    }
+}
diff --git a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/selection/MoveCellSelectionCommandHandler.java b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/selection/MoveCellSelectionCommandHandler.java
index 54ffb687..acf07b5f 100644
--- a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/selection/MoveCellSelectionCommandHandler.java
+++ b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/selection/MoveCellSelectionCommandHandler.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2012, 2020 Original authors and others.
+ * Copyright (c) 2012, 2022 Original authors and others.
  *
  * This program and the accompanying materials are made
  * available under the terms of the Eclipse Public License 2.0
@@ -153,17 +153,11 @@ public class MoveCellSelectionCommandHandler extends MoveSelectionCommandHandler
                                         this.selectionLayer.clear(false);
                                     }
 
-                                    this.selectionLayer.selectCell(
+                                    selectCell(
                                             this.newSelectedColumnPosition,
                                             this.newSelectedRowPosition,
                                             withShiftMask,
                                             withControlMask);
-                                    this.selectionLayer.fireCellSelectionEvent(
-                                            this.newSelectedColumnPosition,
-                                            this.newSelectedRowPosition,
-                                            true,
-                                            withShiftMask,
-                                            withControlMask);
 
                                     break;
                                 }
@@ -186,15 +180,9 @@ public class MoveCellSelectionCommandHandler extends MoveSelectionCommandHandler
                             this.selectionLayer.clear(false);
                         }
 
-                        this.selectionLayer.selectCell(
-                                this.newSelectedColumnPosition,
-                                this.newSelectedRowPosition,
-                                withShiftMask,
-                                withControlMask);
-                        this.selectionLayer.fireCellSelectionEvent(
+                        selectCell(
                                 this.newSelectedColumnPosition,
                                 this.newSelectedRowPosition,
-                                true,
                                 withShiftMask,
                                 withControlMask);
                     }
@@ -285,17 +273,11 @@ public class MoveCellSelectionCommandHandler extends MoveSelectionCommandHandler
                                         this.selectionLayer.clear(false);
                                     }
 
-                                    this.selectionLayer.selectCell(
+                                    selectCell(
                                             this.newSelectedColumnPosition,
                                             this.newSelectedRowPosition,
                                             withShiftMask,
                                             withControlMask);
-                                    this.selectionLayer.fireCellSelectionEvent(
-                                            this.newSelectedColumnPosition,
-                                            this.newSelectedRowPosition,
-                                            true,
-                                            withShiftMask,
-                                            withControlMask);
 
                                     break;
                                 }
@@ -318,17 +300,11 @@ public class MoveCellSelectionCommandHandler extends MoveSelectionCommandHandler
                             this.selectionLayer.clear(false);
                         }
 
-                        this.selectionLayer.selectCell(
+                        selectCell(
                                 this.newSelectedColumnPosition,
                                 this.newSelectedRowPosition,
                                 withShiftMask,
                                 withControlMask);
-                        this.selectionLayer.fireCellSelectionEvent(
-                                this.newSelectedColumnPosition,
-                                this.newSelectedRowPosition,
-                                true,
-                                withShiftMask,
-                                withControlMask);
                     }
                 }
             }
@@ -412,17 +388,11 @@ public class MoveCellSelectionCommandHandler extends MoveSelectionCommandHandler
                                         this.selectionLayer.clear(false);
                                     }
 
-                                    this.selectionLayer.selectCell(
+                                    selectCell(
                                             this.newSelectedColumnPosition,
                                             this.newSelectedRowPosition,
                                             withShiftMask,
                                             withControlMask);
-                                    this.selectionLayer.fireCellSelectionEvent(
-                                            this.newSelectedColumnPosition,
-                                            this.newSelectedRowPosition,
-                                            true,
-                                            withShiftMask,
-                                            withControlMask);
 
                                     break;
                                 }
@@ -445,15 +415,9 @@ public class MoveCellSelectionCommandHandler extends MoveSelectionCommandHandler
                             this.selectionLayer.clear(false);
                         }
 
-                        this.selectionLayer.selectCell(
-                                this.newSelectedColumnPosition,
-                                this.newSelectedRowPosition,
-                                withShiftMask,
-                                withControlMask);
-                        this.selectionLayer.fireCellSelectionEvent(
+                        selectCell(
                                 this.newSelectedColumnPosition,
                                 this.newSelectedRowPosition,
-                                true,
                                 withShiftMask,
                                 withControlMask);
                     }
@@ -546,15 +510,9 @@ public class MoveCellSelectionCommandHandler extends MoveSelectionCommandHandler
                                         this.selectionLayer.clear(false);
                                     }
 
-                                    this.selectionLayer.selectCell(
-                                            this.newSelectedColumnPosition,
-                                            this.newSelectedRowPosition,
-                                            withShiftMask,
-                                            withControlMask);
-                                    this.selectionLayer.fireCellSelectionEvent(
+                                    selectCell(
                                             this.newSelectedColumnPosition,
                                             this.newSelectedRowPosition,
-                                            true,
                                             withShiftMask,
                                             withControlMask);
 
@@ -579,17 +537,11 @@ public class MoveCellSelectionCommandHandler extends MoveSelectionCommandHandler
                             this.selectionLayer.clear(false);
                         }
 
-                        this.selectionLayer.selectCell(
+                        selectCell(
                                 this.newSelectedColumnPosition,
                                 this.newSelectedRowPosition,
                                 withShiftMask,
                                 withControlMask);
-                        this.selectionLayer.fireCellSelectionEvent(
-                                this.newSelectedColumnPosition,
-                                this.newSelectedRowPosition,
-                                true,
-                                withShiftMask,
-                                withControlMask);
                     }
                 }
             }
@@ -643,6 +595,36 @@ public class MoveCellSelectionCommandHandler extends MoveSelectionCommandHandler
                 || this.newSelectedRowPosition != this.lastSelectedCellPosition.rowPosition);
     }
 
+    /**
+     * Select the cell at the given coordinates according to the modifier key
+     * mask settings.
+     *
+     * @param columnPosition
+     *            the column position of the cell that should be selected
+     * @param rowPosition
+     *            the row position of the cell that should be selected
+     * @param withShiftMask
+     *            boolean flag to indicate whether the shift key modifier is
+     *            enabled or not
+     * @param withControlMask
+     *            boolean flag to indicate whether the control key modifier is
+     *            enabled or not
+     */
+    void selectCell(int columnPosition, int rowPosition, boolean withShiftMask, boolean withControlMask) {
+        this.selectionLayer.selectCell(
+                columnPosition,
+                rowPosition,
+                withShiftMask,
+                withControlMask);
+
+        this.selectionLayer.fireCellSelectionEvent(
+                columnPosition,
+                rowPosition,
+                true,
+                withShiftMask,
+                withControlMask);
+    }
+
     @Override
     public Class<MoveSelectionCommand> getCommandClass() {
         return MoveSelectionCommand.class;
diff --git a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/selection/MoveRowSelectionCommandHandler.java b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/selection/MoveRowSelectionCommandHandler.java
index 9e1203c1..9d9ca24e 100644
--- a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/selection/MoveRowSelectionCommandHandler.java
+++ b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/selection/MoveRowSelectionCommandHandler.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2012, 2020 Original authors and others.
+ * Copyright (c) 2012, 2022 Original authors and others.
  *
  * This program and the accompanying materials are made
  * available under the terms of the Eclipse Public License 2.0
@@ -13,6 +13,8 @@
  ******************************************************************************/
 package org.eclipse.nebula.widgets.nattable.selection;
 
+import org.eclipse.nebula.widgets.nattable.coordinate.PositionCoordinate;
+
 /**
  * Preserves the basic semantics of the cell selection. Additionally it selects
  * the entire row when a cell in the row is selected.
@@ -92,9 +94,23 @@ public class MoveRowSelectionCommandHandler extends MoveCellSelectionCommandHand
     @Override
     protected void moveLastSelectedUp(ITraversalStrategy traversalStrategy, boolean withShiftMask, boolean withControlMask) {
         if (this.selectionLayer.hasRowSelection()) {
+
+            PositionCoordinate anchor = new PositionCoordinate(this.selectionLayer.getSelectionAnchor());
+            PositionCoordinate from = this.selectionLayer.getCellPositionToMoveFrom(withShiftMask, withControlMask);
+            boolean deselect = this.selectionLayer.isRowPositionSelected(from.rowPosition) && this.selectionLayer.isRowPositionSelected(from.rowPosition - 1) && withShiftMask;
+            if (deselect) {
+                // deselect
+                this.selectionLayer.selectRow(this.lastSelectedCellPosition.columnPosition, from.rowPosition, false, true);
+            }
+
             super.moveLastSelectedUp(traversalStrategy, withShiftMask, withControlMask);
 
-            if (this.lastSelectedCellPosition != null) {
+            if (deselect) {
+                // keep the selection anchor at its original position
+                this.selectionLayer.setSelectionAnchor(anchor.columnPosition, anchor.rowPosition);
+            }
+
+            if (!deselect && this.lastSelectedCellPosition != null) {
                 this.selectionLayer.selectRow(
                         this.lastSelectedCellPosition.columnPosition, this.lastSelectedCellPosition.rowPosition,
                         withShiftMask, withControlMask);
@@ -105,9 +121,23 @@ public class MoveRowSelectionCommandHandler extends MoveCellSelectionCommandHand
     @Override
     protected void moveLastSelectedDown(ITraversalStrategy traversalStrategy, boolean withShiftMask, boolean withControlMask) {
         if (this.selectionLayer.hasRowSelection()) {
+
+            PositionCoordinate anchor = new PositionCoordinate(this.selectionLayer.getSelectionAnchor());
+            PositionCoordinate from = this.selectionLayer.getCellPositionToMoveFrom(withShiftMask, withControlMask);
+            boolean deselect = this.selectionLayer.isRowPositionSelected(from.rowPosition) && this.selectionLayer.isRowPositionSelected(from.rowPosition + 1) && withShiftMask;
+            if (deselect) {
+                // deselect
+                this.selectionLayer.selectRow(this.lastSelectedCellPosition.columnPosition, from.rowPosition, false, true);
+            }
+
             super.moveLastSelectedDown(traversalStrategy, withShiftMask, withControlMask);
 
-            if (this.lastSelectedCellPosition != null) {
+            if (deselect) {
+                // keep the selection anchor at its original position
+                this.selectionLayer.setSelectionAnchor(anchor.columnPosition, anchor.rowPosition);
+            }
+
+            if (!deselect && this.lastSelectedCellPosition != null) {
                 this.selectionLayer.selectRow(
                         this.lastSelectedCellPosition.columnPosition, this.lastSelectedCellPosition.rowPosition,
                         withShiftMask, withControlMask);
@@ -115,4 +145,15 @@ public class MoveRowSelectionCommandHandler extends MoveCellSelectionCommandHand
         }
     }
 
+    @Override
+    void selectCell(int columnPosition, int rowPosition, boolean withShiftMask, boolean withControlMask) {
+        // ignore the given column position and stick with the selection anchor
+        // and don't fire a CellSelectionEvent to avoid column scrolling
+        // the required selection event is based on the row selection afterwards
+        this.selectionLayer.selectCell(
+                this.selectionLayer.getSelectionAnchor().columnPosition,
+                rowPosition,
+                withShiftMask,
+                withControlMask);
+    }
 }