Clean up cell selection behavior (#3414)

* Add tests for shift-clicking to extend selection.

* Refactor some tests using the selected utility function.

* Mouseup when not dragging should deselect cells.

* Mousedown should *not* deselect cells.

* Rename notebook isSelected -> isSelectedOrActive and make a new isSelected querying just the selected state.

* Add a contiguous selection framework to a notebook widget.

* Add a mouseup handler to a notebook to deselect cells.

* Add more tests for current behavior around extending selections.

* Add extendContiguousSelectionTo tests.

* Cleanup comment

* Ensure that selectionChanged signal triggered only when necessary.

* Fix bug with separating notions of active and selected for cell removal.

* Clean up tests

* Clean up documentation on the extend selection function.

* Break up the extendContiguousSelectionTo tests.

* Add tests for getContiguousSelection.

* Extend selection (and move active cell) on mouseup rather than mousedown.

There are still some subtle issues with starting a drag/drop we need to work through.

* Unnecessary else.

* More cleanup.

* Missing semicolon.

* Cleanup comment.

* Be careful about when to call the various event handlers.

We need to use currentTarget, not target.

* If we click outside of the cells, just deselect all of the cells.

* Select dropped cells.

* Clean up event handling around mouse down/move/up events.

* When shift-clicking, handle contiguous selection errors.

1. Prevent text selection right off.
2. If we don’t have a contiguous selection, deselect everything and write an error to the console.

* Fix the active cell index when a cell moves.

We were not considering the case when a cell moves from before to behind the active cell, or vice versa.

* Add comprehensive onCellMoved tests for active cell index updates.

* Delete obsolete mouseup test.

* Fix lint errors.

packages/notebook/src/actions.ts

@@ -146,7 +146,7 @@ namespace NotebookActions {
 
     // Get the cells to merge.
     each(widget.widgets, (child, i) => {
-      if (widget.isSelected(child)) {
+      if (widget.isSelectedOrActive(child)) {
         toMerge.push(child.model.value.text);
         if (i !== index) {
           toDelete.push(child.model);
@@ -281,8 +281,8 @@ namespace NotebookActions {
     let widgets = widget.widgets;
     cells.beginCompoundOperation();
     for (let i = cells.length - 2; i > -1; i--) {
-      if (widget.isSelected(widgets[i])) {
-        if (!widget.isSelected(widgets[i + 1])) {
+      if (widget.isSelectedOrActive(widgets[i])) {
+        if (!widget.isSelectedOrActive(widgets[i + 1])) {
           cells.move(i, i + 1);
           if (widget.activeCellIndex === i) {
             widget.activeCellIndex++;
@@ -311,8 +311,8 @@ namespace NotebookActions {
     let widgets = widget.widgets;
     cells.beginCompoundOperation();
     for (let i = 1; i < cells.length; i++) {
-      if (widget.isSelected(widgets[i])) {
-        if (!widget.isSelected(widgets[i - 1])) {
+      if (widget.isSelectedOrActive(widgets[i])) {
+        if (!widget.isSelectedOrActive(widgets[i - 1])) {
           cells.move(i, i - 1);
           if (widget.activeCellIndex === i) {
             widget.activeCellIndex--;
@@ -592,20 +592,7 @@ namespace NotebookActions {
     }
     let state = Private.getState(widget);
     widget.mode = 'command';
-    let current = widget.activeCell;
-    let prev = widget.widgets[widget.activeCellIndex - 1];
-    if (widget.isSelected(prev)) {
-      widget.deselect(current);
-      if (widget.activeCellIndex > 1) {
-        let prevPrev = widget.widgets[widget.activeCellIndex - 2];
-        if (!widget.isSelected(prevPrev)) {
-          widget.deselect(prev);
-        }
-      }
-    } else {
-      widget.select(current);
-    }
-    widget.activeCellIndex -= 1;
+    widget.extendContiguousSelectionTo(widget.activeCellIndex - 1);
     Private.handleState(widget, state, true);
   }
 
@@ -629,20 +616,7 @@ namespace NotebookActions {
     }
     let state = Private.getState(widget);
     widget.mode = 'command';
-    let current = widget.activeCell;
-    let next = widget.widgets[widget.activeCellIndex + 1];
-    if (widget.isSelected(next)) {
-      widget.deselect(current);
-      if (widget.activeCellIndex < widget.model.cells.length - 2) {
-        let nextNext = widget.widgets[widget.activeCellIndex + 2];
-        if (!widget.isSelected(nextNext)) {
-          widget.deselect(next);
-        }
-      }
-    } else {
-      widget.select(current);
-    }
-    widget.activeCellIndex += 1;
+    widget.extendContiguousSelectionTo(widget.activeCellIndex + 1);
     Private.handleState(widget, state, true);
   }
 
@@ -760,7 +734,7 @@ namespace NotebookActions {
         const toDelete: number[] = [];
         each(widget.widgets, (child, i) => {
           let deletable = child.model.metadata.get('deletable');
-          if (widget.isSelected(child) && deletable !== false) {
+          if (widget.isSelectedOrActive(child) && deletable !== false) {
             toDelete.push(i);
           }
         });
@@ -872,7 +846,7 @@ namespace NotebookActions {
     let i = 0;
     each(cells, (cell: ICodeCellModel) => {
       let child = widget.widgets[i];
-      if (widget.isSelected(child) && cell.type === 'code') {
+      if (widget.isSelectedOrActive(child) && cell.type === 'code') {
         cell.outputs.clear();
         (child as CodeCell).outputHidden = false;
         cell.executionCount = null;
@@ -922,7 +896,7 @@ namespace NotebookActions {
     let state = Private.getState(widget);
     let cells = widget.widgets;
     each(cells, (cell: Cell) => {
-      if (widget.isSelected(cell) && cell.model.type === 'code') {
+      if (widget.isSelectedOrActive(cell) && cell.model.type === 'code') {
         cell.inputHidden = true;
       }
     });
@@ -942,7 +916,7 @@ namespace NotebookActions {
     let state = Private.getState(widget);
     let cells = widget.widgets;
     each(cells, (cell: Cell) => {
-      if (widget.isSelected(cell) && cell.model.type === 'code') {
+      if (widget.isSelectedOrActive(cell) && cell.model.type === 'code') {
         cell.inputHidden = false;
       }
     });
@@ -1002,7 +976,7 @@ namespace NotebookActions {
     let state = Private.getState(widget);
     let cells = widget.widgets;
     each(cells, (cell: Cell) => {
-      if (widget.isSelected(cell) && cell.model.type === 'code') {
+      if (widget.isSelectedOrActive(cell) && cell.model.type === 'code') {
         (cell as CodeCell).inputHidden = true;
       }
     });
@@ -1022,7 +996,7 @@ namespace NotebookActions {
     let state = Private.getState(widget);
     let cells = widget.widgets;
     each(cells, (cell: Cell) => {
-      if (widget.isSelected(cell) && cell.model.type === 'code') {
+      if (widget.isSelectedOrActive(cell) && cell.model.type === 'code') {
         (cell as CodeCell).inputHidden = false;
       }
     });
@@ -1093,7 +1067,7 @@ namespace NotebookActions {
     let cells = widget.model.cells;
     let i = 0;
     each(widget.widgets, (child: MarkdownCell) => {
-      if (widget.isSelected(child)) {
+      if (widget.isSelectedOrActive(child)) {
         Private.setMarkdownHeader(cells.get(i), level);
       }
       i++;
@@ -1235,7 +1209,7 @@ namespace Private {
     let lastIndex = widget.activeCellIndex;
     let i = 0;
     each(widget.widgets, child => {
-      if (widget.isSelected(child)) {
+      if (widget.isSelectedOrActive(child)) {
         selected.push(child);
         lastIndex = i;
       }
@@ -1355,7 +1329,7 @@ namespace Private {
     clipboard.clear();
     let data: nbformat.IBaseCell[] = [];
     each(widget.widgets, child => {
-      if (widget.isSelected(child)) {
+      if (widget.isSelectedOrActive(child)) {
         data.push(child.model.toJSON());
       }
     });
@@ -1388,7 +1362,7 @@ namespace Private {
 
     cells.beginCompoundOperation();
     each(widget.widgets, (child, i) => {
-      if (!widget.isSelected(child)) {
+      if (!widget.isSelectedOrActive(child)) {
         return;
       }
       if (child.model.type !== value) {
@@ -1442,7 +1416,7 @@ namespace Private {
     // Find the cells to delete.
     each(widget.widgets, (child, i) => {
       let deletable = child.model.metadata.get('deletable');
-      if (widget.isSelected(child) && deletable !== false) {
+      if (widget.isSelectedOrActive(child) && deletable !== false) {
         toDelete.push(i);
       }
     });

packages/notebook/src/celltools.ts

@@ -124,7 +124,7 @@ class CellTools extends Widget {
       return selected;
     }
     each(panel.notebook.widgets, widget => {
-      if (panel.notebook.isSelected(widget)) {
+      if (panel.notebook.isSelectedOrActive(widget)) {
         selected.push(widget);
       }
     });

packages/notebook/src/default-toolbar.ts

@@ -315,7 +315,7 @@ class CellTypeSwitcher extends Widget {
     let mType: string = widget.activeCell.model.type;
     for (let i = 0; i < widget.widgets.length; i++) {
       let child = widget.widgets[i];
-      if (widget.isSelected(child)) {
+      if (widget.isSelectedOrActive(child)) {
         if (child.model.type !== mType) {
           mType = '-';
           select.appendChild(this._wildCard);

packages/notebook/src/widget.ts

@@ -907,9 +907,16 @@ class Notebook extends StaticNotebook {
   }
 
   /**
-   * Whether a cell is selected or is the active cell.
+   * Whether a cell is selected.
    */
   isSelected(widget: Cell): boolean {
+    return Private.selectedProperty.get(widget);
+  }
+
+  /**
+   * Whether a cell is selected or is the active cell.
+   */
+  isSelectedOrActive(widget: Cell): boolean {
     if (widget === this._activeCell) {
       return true;
     }
@@ -935,6 +942,138 @@ class Notebook extends StaticNotebook {
     this.update();
   }
 
+  /**
+   * Move the head of an existing contiguous selection to extend the selection.
+   *
+   * @param index - The new head of the existing selection.
+   *
+   * #### Notes
+   * If there is no existing selection, the active cell is considered an
+   * existing one-cell selection.
+   *
+   * If the new selection is a single cell, that cell becomes the active cell
+   * and all cells are deselected.
+   *
+   * There is no change if there are no cells (i.e., activeCellIndex is -1).
+   */
+  extendContiguousSelectionTo(index: number): void {
+    let {head, anchor} = this.getContiguousSelection();
+    let i: number;
+
+    // Handle the case of no current selection.
+    if (anchor === null) {
+      if (index === this.activeCellIndex) {
+        // Already collapsed selection, nothing more to do.
+        return;
+      }
+
+      // We will start a new selection below.
+      head = this.activeCellIndex;
+      anchor = this.activeCellIndex;
+    }
+
+    // Move the active cell. We do this before the collapsing shortcut below.
+    this.activeCellIndex = index;
+
+    // Make sure the index is valid, according to the rules for setting and clipping the
+    // active cell index. This may change the index.
+    index = this.activeCellIndex;
+
+    // Collapse the selection if it is only the active cell.
+    if (index === anchor) {
+      this.deselectAll();
+      return;
+    }
+
+    let selectionChanged = false;
+
+    if (head < index) {
+      if (head < anchor) {
+        Private.selectedProperty.set(this.widgets[head], false);
+        selectionChanged = true;
+      }
+
+      // Toggle everything strictly between head and index except anchor.
+      for (i = head + 1; i < index; i++) {
+        if (i !== anchor) {
+          Private.selectedProperty.set(this.widgets[i], !Private.selectedProperty.get(this.widgets[i]));
+          selectionChanged = true;
+        }
+      }
+
+    } else if (index < head) {
+      if (anchor < head) {
+        Private.selectedProperty.set(this.widgets[head], false);
+        selectionChanged = true;
+      }
+
+      // Toggle everything strictly between index and head except anchor.
+      for (i = index + 1; i < head; i++) {
+        if (i !== anchor) {
+          Private.selectedProperty.set(this.widgets[i], !Private.selectedProperty.get(this.widgets[i]));
+          selectionChanged = true;
+        }
+      }
+    }
+
+    // Anchor and index should *always* be selected.
+    if (!Private.selectedProperty.get(this.widgets[anchor])) {
+      selectionChanged = true;
+    }
+    Private.selectedProperty.set(this.widgets[anchor], true);
+
+    if (!Private.selectedProperty.get(this.widgets[index])) {
+      selectionChanged = true;
+    }
+    Private.selectedProperty.set(this.widgets[index], true);
+
+    if (selectionChanged) {
+      this._selectionChanged.emit(void 0);
+    }
+  }
+
+  /**
+   * Get the head and anchor of a contiguous cell selection.
+   *
+   * The head of a contiguous selection is always the active cell.
+   *
+   * If there are no cells selected, `{head: null, anchor: null}` is returned.
+   *
+   * Throws an error if the currently selected cells do not form a contiguous
+   * selection.
+   */
+  getContiguousSelection(): {head: number | null, anchor: number | null} {
+    let cells = this.widgets;
+    let first = ArrayExt.findFirstIndex(cells, c => this.isSelected(c));
+
+    // Return early if no cells are selected.
+    if (first === -1) {
+      return {head: null, anchor: null};
+    }
+
+    let last = ArrayExt.findLastIndex(cells, c => this.isSelected(c), -1, first);
+
+    // Check that the selection is contiguous.
+    for (let i = first; i <= last; i++) {
+      if (!this.isSelected(cells[i])) {
+        throw new Error('Selection not contiguous');
+      }
+    }
+
+    // Check that the active cell is one of the endpoints of the selection.
+    let activeIndex = this.activeCellIndex;
+    if (first !== activeIndex && last !== activeIndex) {
+      throw new Error('Active cell not at endpoint of selection');
+    }
+
+    // Determine the head and anchor of the selection.
+    if (first === activeIndex) {
+      return {head: first, anchor: last};
+    } else {
+      return {head: last, anchor: first};
+    }
+  }
+
   /**
    * Scroll so that the given position is visible.
    *
@@ -971,10 +1110,14 @@ class Notebook extends StaticNotebook {
       this._evtMouseDown(event as MouseEvent);
       break;
     case 'mouseup':
-      this._evtMouseup(event as MouseEvent);
+      if (event.currentTarget === document) {
+        this._evtDocumentMouseup(event as MouseEvent);
+      }
       break;
     case 'mousemove':
-      this._evtMousemove(event as MouseEvent);
+      if (event.currentTarget === document) {
+        this._evtDocumentMousemove(event as MouseEvent);
+      }
       break;
     case 'keydown':
       this._ensureFocus(true);
@@ -1071,7 +1214,7 @@ class Notebook extends StaticNotebook {
         widget.removeClass(ACTIVE_CLASS);
       }
       widget.removeClass(OTHER_SELECTED_CLASS);
-      if (this.isSelected(widget)) {
+      if (this.isSelectedOrActive(widget)) {
         widget.addClass(SELECTED_CLASS);
         count++;
       } else {
@@ -1112,8 +1255,13 @@ class Notebook extends StaticNotebook {
    * Handle a cell being moved.
    */
   protected onCellMoved(fromIndex: number, toIndex: number): void {
-    if (fromIndex === this.activeCellIndex) {
+    let i = this.activeCellIndex;
+    if (fromIndex === i) {
       this.activeCellIndex = toIndex;
+    } else if (fromIndex < i && i <= toIndex) {
+      this.activeCellIndex--;
+    } else if (toIndex <= i && i < fromIndex) {
+      this.activeCellIndex++;
     }
   }
 
@@ -1203,55 +1351,75 @@ class Notebook extends StaticNotebook {
    * Handle `mousedown` events for the widget.
    */
   private _evtMouseDown(event: MouseEvent): void {
+
+    // Mouse click should always ensure the notebook is focused.
+    this._ensureFocus(true);
+
+    // We only handle main button actions.
+    if (event.button !== 0) {
+      return;
+    }
+
+    // Try to find the cell associated with the event.
     let target = event.target as HTMLElement;
-    let i = this._findCell(target);
-    let shouldDrag = false;
+    let index = this._findCell(target);
+
+    if (index !== -1) {
+      let widget = this.widgets[index];
 
-    if (i !== -1) {
-      let widget = this.widgets[i];
       // Event is on a cell but not in its editor, switch to command mode.
       if (!widget.editorWidget.node.contains(target)) {
         this.mode = 'command';
-        shouldDrag = widget.promptNode.contains(target);
       }
-      if (event.shiftKey) {
-        shouldDrag = false;
-        this._extendSelectionTo(i);
 
+      if (event.shiftKey) {
         // Prevent text select behavior.
         event.preventDefault();
         event.stopPropagation();
+
+        try {
+          this.extendContiguousSelectionTo(index);
+        } catch (e) {
+          console.error(e);
+          this.deselectAll();
+          return;
+        }
+        // Enter selecting mode
+        this._mouseMode = 'select';
+        document.addEventListener('mouseup', this, true);
+        document.addEventListener('mousemove', this, true);
+
       } else {
-        if (!this.isSelected(widget)) {
+        // Check if we need to change the active cell.
+        if (!this.isSelectedOrActive(widget)) {
           this.deselectAll();
+          this.activeCellIndex = index;
         }
-      }
-      // Set the cell as the active one.
-      // This must be done *after* setting the mode above.
-      this.activeCellIndex = i;
-    }
 
-    this._ensureFocus(true);
+        // Prepare to start a drag if we are on the drag region.
+        if (widget.promptNode.contains(target)) {
+          // Prepare for a drag start
+          this._dragData = { pressX: event.clientX, pressY: event.clientY, index: index};
+
+          // Enter possible drag mode
+          this._mouseMode = 'couldDrag';
+          document.addEventListener('mouseup', this, true);
+          document.addEventListener('mousemove', this, true);
+        }
+        event.preventDefault();
+      }
 
-    // Left mouse press for drag start.
-    if (event.button === 0 && shouldDrag) {
-      this._dragData = { pressX: event.clientX, pressY: event.clientY, index: i};
-      document.addEventListener('mouseup', this, true);
-      document.addEventListener('mousemove', this, true);
-      event.preventDefault();
     }
   }
 
-
   /**
-   * Handle the `'mouseup'` event for the widget.
+   * Handle the `'mouseup'` event on the document.
    */
-  private _evtMouseup(event: MouseEvent): void {
-    if (event.button !== 0 || !this._drag) {
-      document.removeEventListener('mousemove', this, true);
-      document.removeEventListener('mouseup', this, true);
-      return;
-    }
+  private _evtDocumentMouseup(event: MouseEvent): void {
+    // Remove the event listeners we put on the document
+    document.removeEventListener('mousemove', this, true);
+    document.removeEventListener('mouseup', this, true);
+    this._mouseMode = null;
     event.preventDefault();
     event.stopPropagation();
   }
@@ -1259,24 +1427,33 @@ class Notebook extends StaticNotebook {
   /**
    * Handle the `'mousemove'` event for the widget.
    */
-  private _evtMousemove(event: MouseEvent): void {
+  private _evtDocumentMousemove(event: MouseEvent): void {
+
     event.preventDefault();
     event.stopPropagation();
 
-    // Bail if we are the one dragging.
-    if (this._drag) {
-      return;
-    }
-
-    // Check for a drag initialization.
-    let data = this._dragData;
-    let dx = Math.abs(event.clientX - data.pressX);
-    let dy = Math.abs(event.clientY - data.pressY);
-    if (dx < DRAG_THRESHOLD && dy < DRAG_THRESHOLD) {
-      return;
+    // If in select mode, update the selection
+    switch (this._mouseMode) {
+    case 'select':
+      let target = event.target as HTMLElement;
+      let index = this._findCell(target);
+      if (index !== -1) {
+        this.extendContiguousSelectionTo(index);
+      }
+      break;
+    case 'couldDrag':
+      // Check for a drag initialization.
+      let data = this._dragData;
+      let dx = Math.abs(event.clientX - data.pressX);
+      let dy = Math.abs(event.clientY - data.pressY);
+      if (dx >= DRAG_THRESHOLD || dy >= DRAG_THRESHOLD) {
+        this._mouseMode = null;
+        this._startDrag(data.index, event.clientX, event.clientY);
+      }
+      break;
+    default:
+      break;
     }
-
-    this._startDrag(data.index, event.clientX, event.clientY);
   }
 
   /**
@@ -1382,6 +1559,7 @@ class Notebook extends StaticNotebook {
       if (toIndex >= fromIndex && toIndex < fromIndex + toMove.length) {
         return;
       }
+
       // Move the cells one by one
       this.model.cells.beginCompoundOperation();
       if (fromIndex < toIndex) {
@@ -1403,6 +1581,7 @@ class Notebook extends StaticNotebook {
       if (index === -1) {
         index = this.widgets.length;
       }
+      let start = index;
       let model = this.model;
       let values = event.mimeData.getData(JUPYTER_CELL_MIME);
       let factory = model.contentFactory;
@@ -1425,8 +1604,10 @@ class Notebook extends StaticNotebook {
         model.cells.insert(index++, value);
       });
       model.cells.endCompoundOperation();
-      // Activate the last cell.
-      this.activeCellIndex = index - 1;
+      // Select the inserted cells.
+      this.deselectAll();
+      this.activeCellIndex = start;
+      this.extendContiguousSelectionTo(index - 1);
     }
   }
 
@@ -1440,7 +1621,7 @@ class Notebook extends StaticNotebook {
 
     each(this.widgets, (widget, i) => {
       let cell = cells.get(i);
-      if (this.isSelected(widget)) {
+      if (this.isSelectedOrActive(widget)) {
         widget.addClass(DROP_SOURCE_CLASS);
         selected.push(cell.toJSON());
         toMove.push(widget);
@@ -1479,6 +1660,7 @@ class Notebook extends StaticNotebook {
     // Remove mousemove and mouseup listeners and start the drag.
     document.removeEventListener('mousemove', this, true);
     document.removeEventListener('mouseup', this, true);
+    this._mouseMode = null;
     this._drag.start(clientX, clientY).then(action => {
       if (this.isDisposed) {
         return;
@@ -1547,28 +1729,6 @@ class Notebook extends StaticNotebook {
     }
   }
 
-  /**
-   * Extend the selection to a given index.
-   */
-  private _extendSelectionTo(index: number): void {
-    let activeIndex = this.activeCellIndex;
-    let j = index;
-    // extend the existing selection.
-    if (j > activeIndex) {
-      while (j > activeIndex) {
-        Private.selectedProperty.set(this.widgets[j], true);
-        j--;
-      }
-    } else if (j < activeIndex) {
-      while (j < activeIndex) {
-        Private.selectedProperty.set(this.widgets[j], true);
-        j++;
-      }
-    }
-    Private.selectedProperty.set(this.widgets[activeIndex], true);
-    this._selectionChanged.emit(void 0);
-  }
-
   /**
    * Remove selections from inactive cells to avoid
    * spurious cursors.
@@ -1588,6 +1748,7 @@ class Notebook extends StaticNotebook {
   private _mode: NotebookMode = 'command';
   private _drag: Drag = null;
   private _dragData: { pressX: number, pressY: number, index: number } = null;
+  private _mouseMode: 'select' | 'couldDrag' | null = null;
   private _activeCellChanged = new Signal<this, Cell>(this);
   private _stateChanged = new Signal<this, IChangedArgs<any>>(this);
   private _selectionChanged = new Signal<this, void>(this);

test/src/notebook/actions.spec.ts

@@ -887,7 +887,7 @@ describe('@jupyterlab/notebook', () => {
         expect(widget.activeCellIndex).to.be(-1);
       });
 
-      it('should change to command mode', () => {
+      it('should change to command mode if there is a selection', () => {
         widget.mode = 'edit';
         widget.activeCellIndex = 1;
         NotebookActions.extendSelectionAbove(widget);
@@ -895,10 +895,12 @@ describe('@jupyterlab/notebook', () => {
       });
 
       it('should not wrap around to the bottom', () => {
+        widget.mode = 'edit';
         NotebookActions.extendSelectionAbove(widget);
         expect(widget.activeCellIndex).to.be(0);
         let last = widget.widgets[widget.widgets.length - 1];
         expect(widget.isSelected(last)).to.be(false);
+        expect(widget.mode).to.equal('edit');
       });
 
       it('should deselect the current cell if the cell above is selected', () => {
@@ -939,18 +941,20 @@ describe('@jupyterlab/notebook', () => {
         expect(widget.activeCellIndex).to.be(-1);
       });
 
-      it('should change to command mode', () => {
+      it('should change to command mode if there is a selection', () => {
         widget.mode = 'edit';
         NotebookActions.extendSelectionBelow(widget);
         expect(widget.mode).to.be('command');
       });
 
-      it('should not wrap around to the bottom', () => {
+      it('should not wrap around to the top', () => {
         let last = widget.widgets.length - 1;
         widget.activeCellIndex = last;
+        widget.mode = 'edit';
         NotebookActions.extendSelectionBelow(widget);
         expect(widget.activeCellIndex).to.be(last);
         expect(widget.isSelected(widget.widgets[0])).to.be(false);
+        expect(widget.mode).to.equal('edit');
       });
 
       it('should deselect the current cell if the cell below is selected', () => {

test/src/notebook/widget.spec.ts

@@ -140,6 +140,18 @@ function createActiveWidget(): LogNotebook {
 }
 
 
+function selected(nb: Notebook): number[] {
+    const selected = [];
+    const cells = nb.widgets;
+    for (let i = 0; i < cells.length; i++) {
+      if (nb.isSelected(cells[i])) {
+        selected.push(i);
+      }
+    }
+    return selected;
+}
+
+
 describe('notebook/widget', () => {
 
   describe('StaticNotebook', () => {
@@ -669,19 +681,11 @@ describe('notebook/widget', () => {
         widget.model.fromJSON(DEFAULT_CONTENT);
         Widget.attach(widget, document.body);
         requestAnimationFrame(() => {
-          for (let i = 0; i < widget.widgets.length; i++) {
-            let cell = widget.widgets[i];
-            widget.select(cell);
-            expect(widget.isSelected(cell)).to.be(true);
-          }
+          widget.extendContiguousSelectionTo(widget.widgets.length - 1);
+          let selectedRange = Array.from(Array(widget.widgets.length).keys());
+          expect(selected(widget)).to.eql(selectedRange);
           widget.mode = 'edit';
-          for (let i = 0; i < widget.widgets.length; i++) {
-            if (i === widget.activeCellIndex) {
-              continue;
-            }
-            let cell = widget.widgets[i];
-            expect(widget.isSelected(cell)).to.be(false);
-          }
+          expect(selected(widget)).to.eql([]);
           widget.dispose();
           done();
         });
@@ -790,11 +794,9 @@ describe('notebook/widget', () => {
       it('should allow multiple widgets to be selected', () => {
         let widget = createActiveWidget();
         widget.model.fromJSON(DEFAULT_CONTENT);
-        for (let i = 0; i < widget.widgets.length; i++) {
-          let cell = widget.widgets[i];
-          widget.select(cell);
-          expect(widget.isSelected(cell)).to.be(true);
-        }
+        widget.widgets.forEach(cell => { widget.select(cell); });
+        let expectSelected = Array.from(Array(widget.widgets.length).keys());
+        expect(selected(widget)).to.eql(expectSelected);
       });
 
     });
@@ -805,9 +807,6 @@ describe('notebook/widget', () => {
         let widget = createActiveWidget();
         widget.model.fromJSON(DEFAULT_CONTENT);
         for (let i = 0; i < widget.widgets.length; i++) {
-          if (i === widget.activeCellIndex) {
-            continue;
-          }
           let cell = widget.widgets[i];
           widget.select(cell);
           expect(widget.isSelected(cell)).to.be(true);
@@ -816,15 +815,17 @@ describe('notebook/widget', () => {
         }
       });
 
-      it('should have no effect on the active cell', () => {
+      it('should let the active cell be deselected', () => {
         let widget = createActiveWidget();
         widget.model.fromJSON(DEFAULT_CONTENT);
-        let cell = widget.widgets[widget.activeCellIndex];
+        let cell = widget.activeCell;
+        widget.select(cell);
         expect(widget.isSelected(cell)).to.be(true);
         widget.deselect(cell);
-        expect(widget.isSelected(cell)).to.be(true);
+        expect(widget.isSelected(cell)).to.be(false);
       });
 
+
     });
 
     describe('#isSelected()', () => {
@@ -832,14 +833,292 @@ describe('notebook/widget', () => {
       it('should get whether the cell is selected', () => {
         let widget = createActiveWidget();
         widget.model.fromJSON(DEFAULT_CONTENT);
-        for (let i = 0; i < widget.widgets.length; i++) {
-          let cell = widget.widgets[i];
-          if (i === widget.activeCellIndex) {
-            expect(widget.isSelected(cell)).to.be(true);
+        widget.select(widget.widgets[0]);
+        widget.select(widget.widgets[2]);
+        expect(selected(widget)).to.eql([0, 2]);
+      });
+
+      it('reports selection whether or not cell is active', () => {
+        let widget = createActiveWidget();
+        widget.model.fromJSON(DEFAULT_CONTENT);
+        expect(selected(widget)).to.eql([]);
+        widget.select(widget.activeCell);
+        expect(selected(widget)).to.eql([widget.activeCellIndex]);
+      });
+
+    });
+
+    describe('#deselectAll()', () => {
+
+      it('should deselect all cells', () => {
+        let widget = createActiveWidget();
+        widget.model.fromJSON(DEFAULT_CONTENT);
+        widget.select(widget.widgets[0]);
+        widget.select(widget.widgets[2]);
+        widget.select(widget.widgets[3]);
+        widget.select(widget.widgets[4]);
+        expect(selected(widget)).to.eql([0, 2, 3, 4]);
+        widget.deselectAll();
+        expect(selected(widget)).to.eql([]);
+      });
+
+    });
+
+    describe('#extendContiguousSelectionTo()', () => {
+
+      // Test a permutation for extending a selection.
+      let checkSelection = (widget: Notebook, anchor: number, head: number, index: number, select = true) => {
+
+        if (!select && anchor !== head) {
+          throw new Error('anchor must equal head if select is false');
+        }
+
+        // Set up the test by pre-selecting appropriate cells if select is true.
+        if (select) {
+          for (let i = Math.min(anchor, head); i <= Math.max(anchor, head); i++) {
+            widget.select(widget.widgets[i]);
+          }
+        }
+
+        // Set the active cell to indicate the head of the selection.
+        widget.activeCellIndex = head;
+
+        // Set up a selection event listener.
+        let selectionChanged = 0;
+        let countSelectionChanged = (sender: Notebook, args: void) => {
+          selectionChanged += 1;
+        };
+        widget.selectionChanged.connect(countSelectionChanged);
+
+        // Check the contiguous selection.
+        let selection = widget.getContiguousSelection();
+        if (select) {
+          expect(selection.anchor).to.be(anchor);
+          expect(selection.head).to.be(head);
+        } else {
+          expect(selection.anchor).to.be(null);
+          expect(selection.head).to.be(null);
+        }
+
+        // Extend the selection.
+        widget.extendContiguousSelectionTo(index);
+
+        // Clip index to fall within the cell index range.
+        index = Math.max(0, Math.min(widget.widgets.length - 1, index));
+
+        // Check the active cell is now at the index.
+        expect(widget.activeCellIndex).to.be.equal(index);
+
+        // Check the contiguous selection.
+        selection = widget.getContiguousSelection();
+
+        // Check the selection changed signal was emitted once if necessary.
+        if (head === index) {
+          if (index === anchor && select) {
+            // we should have collapsed the single cell selection
+            expect(selectionChanged).to.be(1);
           } else {
-            expect(widget.isSelected(cell)).to.be(false);
+            expect(selectionChanged).to.be(0);
           }
+        } else {
+          expect(selectionChanged).to.be(1);
+        }
+
+        if (anchor !== index) {
+          expect(selection.anchor).to.be.equal(anchor);
+          expect(selection.head).to.be.equal(index);
+        } else {
+          // If the anchor and index are the same, the selection is collapsed.
+          expect(selection.anchor).to.be.equal(null);
+          expect(selection.head).to.be.equal(null);
         }
+
+        // Clean up widget
+        widget.selectionChanged.disconnect(countSelectionChanged);
+        widget.activeCellIndex = 0;
+        widget.deselectAll();
+      };
+
+      // Lists are of the form [anchor, head, index].
+      let permutations = [
+        // Anchor, head, and index are distinct
+        [1, 3, 5],
+        [1, 5, 3],
+        [3, 1, 5],
+        [3, 5, 1],
+        [5, 1, 3],
+        [5, 3, 1],
+
+        // Two of anchor, head, and index are equal
+        [1, 3, 3],
+        [3, 1, 3],
+        [3, 3, 1],
+
+        // Anchor, head, and index all equal
+        [3, 3, 3]
+      ];
+
+      it('should work in each permutation of anchor, head, and index', () => {
+        let widget = createActiveWidget();
+        widget.model.fromJSON(DEFAULT_CONTENT);
+
+        permutations.forEach(p => {
+          checkSelection(widget, p[0], p[1], p[2]);
+        });
+      });
+
+      it('should work when we only have an active cell, with no existing selection', () => {
+        let widget = createActiveWidget();
+        widget.model.fromJSON(DEFAULT_CONTENT);
+
+        permutations.forEach(p => {
+          if (p[0] === p[1]) {
+            checkSelection(widget, p[0], p[1], p[2], false);
+          }
+        });
+      });
+
+      it('should clip when the index is greater than the last index', () => {
+        let widget = createActiveWidget();
+        widget.model.fromJSON(DEFAULT_CONTENT);
+
+        permutations.forEach(p => {
+          checkSelection(widget, p[0], p[1], Number.MAX_SAFE_INTEGER);
+        });
+      });
+
+      it('should clip when the index is greater than the last index with no existing selection', () => {
+        let widget = createActiveWidget();
+        widget.model.fromJSON(DEFAULT_CONTENT);
+
+        permutations.forEach(p => {
+          if (p[0] === p[1]) {
+            checkSelection(widget, p[0], p[1], Number.MAX_SAFE_INTEGER, false);
+          }
+        });
+      });
+
+      it('should clip when the index is less than 0', () => {
+        let widget = createActiveWidget();
+        widget.model.fromJSON(DEFAULT_CONTENT);
+
+        permutations.forEach(p => {
+          checkSelection(widget, p[0], p[1], -10);
+        });
+      });
+
+      it('should clip when the index is less than 0 with no existing selection', () => {
+        let widget = createActiveWidget();
+        widget.model.fromJSON(DEFAULT_CONTENT);
+
+        permutations.forEach(p => {
+          if (p[0] === p[1]) {
+            checkSelection(widget, p[0], p[1], -10, false);
+          }
+        });
+      });
+
+      it('handles the case of no cells', () => {
+        let widget = createActiveWidget();
+        widget.model.cells.clear();
+        expect(widget.widgets.length).to.be(0);
+
+        // Set up a selection event listener.
+        let selectionChanged = 0;
+        widget.selectionChanged.connect((sender, args) => {
+          selectionChanged += 1;
+        });
+
+        widget.extendContiguousSelectionTo(3);
+
+        expect(widget.activeCellIndex).to.be(-1);
+        expect(selectionChanged).to.be(0);
+      });
+
+    });
+
+    describe('#getContiguousSelection()', () => {
+
+      it('throws an error when the selection is not contiguous', () => {
+        let widget = createActiveWidget();
+        widget.model.fromJSON(DEFAULT_CONTENT);
+
+        widget.select(widget.widgets[1]);
+        widget.select(widget.widgets[3]);
+        widget.activeCellIndex = 3;
+
+        expect(() => widget.getContiguousSelection()).to.throwError(/Selection not contiguous/);
+      });
+
+      it('throws an error if the active cell is not at an endpoint', () => {
+        let widget = createActiveWidget();
+        widget.model.fromJSON(DEFAULT_CONTENT);
+
+        widget.select(widget.widgets[1]);
+        widget.select(widget.widgets[2]);
+        widget.select(widget.widgets[3]);
+
+        // Check if active cell is outside selection.
+        widget.activeCellIndex = 0;
+        expect(() => widget.getContiguousSelection()).to.throwError(/Active cell not at endpoint of selection/);
+
+        // Check if active cell is inside selection.
+        widget.activeCellIndex = 2;
+        expect(() => widget.getContiguousSelection()).to.throwError(/Active cell not at endpoint of selection/);
+      });
+
+      it('returns null values if there is no selection', () => {
+        let widget = createActiveWidget();
+        widget.model.fromJSON(DEFAULT_CONTENT);
+
+        let selection = widget.getContiguousSelection();
+        expect(selection).to.eql({head: null, anchor: null});
+      });
+
+      it('handles the case of no cells', () => {
+        let widget = createActiveWidget();
+        widget.model.cells.clear();
+        expect(widget.widgets.length).to.be(0);
+
+        let selection = widget.getContiguousSelection();
+        expect(selection).to.eql({head: null, anchor: null});
+      });
+
+      it('works if head is before the anchor', () => {
+        let widget = createActiveWidget();
+        widget.model.fromJSON(DEFAULT_CONTENT);
+
+        widget.select(widget.widgets[1]);
+        widget.select(widget.widgets[2]);
+        widget.select(widget.widgets[3]);
+        widget.activeCellIndex = 1;
+
+        let selection = widget.getContiguousSelection();
+        expect(selection).to.eql({head: 1, anchor: 3});
+      });
+
+      it('works if head is after the anchor', () => {
+        let widget = createActiveWidget();
+        widget.model.fromJSON(DEFAULT_CONTENT);
+
+        widget.select(widget.widgets[1]);
+        widget.select(widget.widgets[2]);
+        widget.select(widget.widgets[3]);
+        widget.activeCellIndex = 3;
+
+        let selection = widget.getContiguousSelection();
+        expect(selection).to.eql({head: 3, anchor: 1});
+      });
+
+      it('works if head and anchor are the same', () => {
+        let widget = createActiveWidget();
+        widget.model.fromJSON(DEFAULT_CONTENT);
+
+        widget.select(widget.widgets[3]);
+        widget.activeCellIndex = 3;
+
+        let selection = widget.getContiguousSelection();
+        expect(selection).to.eql({head: 3, anchor: 3});
       });
 
     });
@@ -865,6 +1144,7 @@ describe('notebook/widget', () => {
           let child = widget.widgets[1];
           simulate(child.node, 'mousedown');
           expect(widget.events).to.contain('mousedown');
+          expect(widget.isSelected(widget.widgets[0])).to.be(false);
           expect(widget.activeCellIndex).to.be(1);
         });
 
@@ -885,6 +1165,30 @@ describe('notebook/widget', () => {
           expect(widget.activeCell).to.be(child);
         });
 
+        it('should extend selection if invoked with shift', () => {
+          widget.activeCellIndex = 3;
+
+          // shift click below
+          simulate(widget.widgets[4].node, 'mousedown', {shiftKey: true});
+          expect(widget.activeCellIndex).to.be(4);
+          expect(selected(widget)).to.eql([3, 4]);
+
+          // shift click above
+          simulate(widget.widgets[1].node, 'mousedown', {shiftKey: true});
+          expect(widget.activeCellIndex).to.be(1);
+          expect(selected(widget)).to.eql([1, 2, 3]);
+
+          // shift click expand
+          simulate(widget.widgets[0].node, 'mousedown', {shiftKey: true});
+          expect(widget.activeCellIndex).to.be(0);
+          expect(selected(widget)).to.eql([0, 1, 2, 3]);
+
+          // shift click contract
+          simulate(widget.widgets[2].node, 'mousedown', {shiftKey: true});
+          expect(widget.activeCellIndex).to.be(2);
+          expect(selected(widget)).to.eql([2, 3]);
+        });
+
       });
 
       context('dblclick', () => {
@@ -1146,9 +1450,29 @@ describe('notebook/widget', () => {
 
       it('should update the active cell index if necessary', () => {
         let widget = createActiveWidget();
-        widget.model.fromJSON(DEFAULT_CONTENT);
-        widget.model.cells.move(1, 0);
-        expect(widget.activeCellIndex).to.be(0);
+
+        // [fromIndex, toIndex, activeIndex], starting with activeIndex=3.
+        let moves = [
+          [0, 2, 3],
+          [0, 3, 2],
+          [0, 4, 2],
+          [3, 2, 2],
+          [3, 3, 3],
+          [3, 4, 4],
+          [4, 2, 4],
+          [4, 3, 4],
+          [4, 5, 3]
+        ];
+
+        moves.forEach((m) => {
+          let [fromIndex, toIndex, activeIndex] = m;
+          widget.model.fromJSON(DEFAULT_CONTENT);
+          let cell = widget.widgets[3];
+          widget.activeCellIndex = 3;
+          widget.model.cells.move(fromIndex, toIndex);
+          expect(widget.activeCellIndex).to.be(activeIndex);
+          expect(widget.widgets[activeIndex]).to.be(cell);
+        });
       });
 
     });