Web Inspector: Table should support shift-extending the row selection
authormattbaker@apple.com <mattbaker@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 13 Nov 2018 05:13:02 +0000 (05:13 +0000)
committermattbaker@apple.com <mattbaker@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 13 Nov 2018 05:13:02 +0000 (05:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=189718
<rdar://problem/44577942>

Reviewed by Devin Rousso.

Source/WebInspectorUI:

Allow the table selection to be extended by shift-clicking a row, or by
holding shift and pressing either the up or down arrow key. If both command
and shift are pressed, shift is ignored. The selection behavior is modeled
after AppKit's NSTableView.

* UserInterface/Base/IndexSet.js:
(WI.IndexSet.prototype.addRange):
(WI.IndexSet.prototype.deleteRange):
(WI.IndexSet.prototype.equals):
(WI.IndexSet.prototype.difference):

* UserInterface/Views/Table.js:
(WI.Table):
(WI.Table.prototype.set allowsMultipleSelection):
(WI.Table.prototype.reloadData):
(WI.Table.prototype.selectRow):
(WI.Table.prototype.deselectRow):
(WI.Table.prototype._handleKeyDown):
Holding shift and pressing either the up or down arrow key extends the
selection to the next unselected row adjacent to the anchor row, or causes
the anchor row to be deselected, decreasing the selection. The table chooses
the action to take based on the direction of movement (up or down), and
the currently selected rows.

(WI.Table.prototype._selectRowsFromArrowKey):
(WI.Table.prototype._handleMouseDown.normalizeRange):
(WI.Table.prototype._handleMouseDown):
Clicking a row while holding down shift extends the selection to include
the rows between the anchor row (exclusive) and clicked row (inclusive).
The anchor row is equal to the value of `_selectedRowIndex` prior to
clicking a new row.

(WI.Table.prototype._deselectAllAndSelect):
(WI.Table.prototype._removeRows):
(WI.Table.prototype._toggleSelectedRowStyle):
(WI.Table.prototype._updateSelectedRows):
Helper method for updating the selection to the specified rows, and updating
DOM styles for rows that are added to or removed from the selection.

LayoutTests:

* inspector/unit-tests/index-set-expected.txt:
* inspector/unit-tests/index-set.html:
Add tests for new IndexSet methods `addRange`, `deleteRange`, `equals`, and `difference`.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238121 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/inspector/unit-tests/index-set-expected.txt
LayoutTests/inspector/unit-tests/index-set.html
Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Base/IndexSet.js
Source/WebInspectorUI/UserInterface/Views/Table.js

index 1fa3e72..c7af5fb 100644 (file)
@@ -1,3 +1,15 @@
+2018-11-12  Matt Baker  <mattbaker@apple.com>
+
+        Web Inspector: Table should support shift-extending the row selection
+        https://bugs.webkit.org/show_bug.cgi?id=189718
+        <rdar://problem/44577942>
+
+        Reviewed by Devin Rousso.
+
+        * inspector/unit-tests/index-set-expected.txt:
+        * inspector/unit-tests/index-set.html:
+        Add tests for new IndexSet methods `addRange`, `deleteRange`, `equals`, and `difference`.
+
 2018-11-12  Zalan Bujtas  <zalan@apple.com>
 
         Do not collapse the soon-to-be-parent anon block when we shuffle around the marker item renderer.
index 81f9a5e..f56a474 100644 (file)
@@ -72,3 +72,82 @@ PASS: Index less than 3 should be 2.
 PASS: Copy and original should be different objects.
 PASS: Copy and original should have the same values.
 
+-- Running test case: IndexSet.prototype.addRange
+Given an IndexSet with values []:
+Add range to an empty IndexSet.
+PASS: Should be [1,2,3] after adding [1,2,3].
+
+Given an IndexSet with values [10,11,12]:
+Add range before the beginning.
+PASS: Should be [0,1,2,10,11,12] after adding [0,1,2].
+
+Given an IndexSet with values [1,2,3]:
+Add range after the end.
+PASS: Should be [1,2,3,10,11,12] after adding [10,11,12].
+
+Given an IndexSet with values [1,5]:
+Add range in the middle.
+PASS: Should be [1,2,3,4,5] after adding [2,3,4].
+
+Given an IndexSet with values [1,3,5]:
+Add range overlapping the middle.
+PASS: Should be [1,2,3,4,5] after adding [2,3,4].
+
+Given an IndexSet with values [3,4,5]:
+Add range overlapping the beginning.
+PASS: Should be [1,2,3,4,5] after adding [1,2,3].
+
+Given an IndexSet with values [1,2,3]:
+Add range overlapping the end.
+PASS: Should be [1,2,3,4,5] after adding [3,4,5].
+
+
+-- Running test case: IndexSet.prototype.deleteRange
+Given an IndexSet with values []:
+Remove range from an empty IndexSet.
+PASS: Should be [] after removing [1,2,3].
+
+Given an IndexSet with values [10,11,12]:
+Remove range before the beginning.
+PASS: Should be [10,11,12] after removing [0,1,2].
+
+Given an IndexSet with values [0,1,2]:
+Remove range after the end.
+PASS: Should be [0,1,2] after removing [10,11,12].
+
+Given an IndexSet with values [0,1,2,3]:
+Remove range in the middle.
+PASS: Should be [0,3] after removing [1,2].
+
+Given an IndexSet with values [1,3,5]:
+Remove range overlapping the middle.
+PASS: Should be [1,5] after removing [2,3,4].
+
+Given an IndexSet with values [1,2,3]:
+Remove range overlapping the beginning.
+PASS: Should be [3] after removing [0,1,2].
+
+Given an IndexSet with values [1,2,3]:
+Remove range overlapping the end.
+PASS: Should be [1] after removing [2,3,4].
+
+
+-- Running test case: IndexSet.prototype.equals
+PASS: Should trivially equal itself.
+PASS: Copy and original should be equal.
+PASS: Modified copy and original should not be equal.
+
+-- Running test case: IndexSet.prototype.difference
+Given an IndexSet with values [], and another IndexSet with values []:
+PASS: Difference between the first and second IndexSet should be [].
+
+Given an IndexSet with values [1,2,3], and another IndexSet with values []:
+PASS: Difference between the first and second IndexSet should be [1,2,3].
+
+Given an IndexSet with values [], and another IndexSet with values [1,2,3]:
+PASS: Difference between the first and second IndexSet should be [].
+
+Given an IndexSet with values [1,2,3], and another IndexSet with values [2,3,4]:
+PASS: Difference between the first and second IndexSet should be [1].
+
+
index 6c64ea8..185e1ad 100644 (file)
@@ -15,6 +15,13 @@ function test()
         return new WI.IndexSet(values);
     }
 
+    function rangeToArray(startIndex, count) {
+        let result = [];
+        for (let i = 0; i < count; ++i)
+            result.push(startIndex + i);
+        return result;
+    }
+
     suite.addTestCase({
         name: "IndexSet.constructor",
         test() {
@@ -206,6 +213,207 @@ function test()
         }
     });
 
+    suite.addTestCase({
+        name: "IndexSet.prototype.addRange",
+        test() {
+            function testAddRange({description, initialValues, startIndex, count, expectedValues}) {
+                let indexSet = createIndexSet(initialValues || []);
+
+                InspectorTest.log(description);
+                indexSet.addRange(startIndex, count);
+                InspectorTest.expectShallowEqual(Array.from(indexSet), expectedValues, `Should be [${expectedValues}] after adding [${rangeToArray(startIndex, count)}].`);
+                InspectorTest.log("");
+            }
+
+            testAddRange({
+                description: "Add range to an empty IndexSet.",
+                initialValues: [],
+                startIndex: 1,
+                count: 3,
+                expectedValues: [1, 2, 3],
+            });
+
+            testAddRange({
+                description: "Add range before the beginning.",
+                initialValues: [10, 11, 12],
+                startIndex: 0,
+                count: 3,
+                expectedValues: [0, 1, 2, 10, 11, 12],
+            });
+
+            testAddRange({
+                description: "Add range after the end.",
+                initialValues: [1, 2, 3],
+                startIndex: 10,
+                count: 3,
+                expectedValues: [1, 2, 3, 10, 11, 12],
+            });
+
+            testAddRange({
+                description: "Add range in the middle.",
+                initialValues: [1, 5],
+                startIndex: 2,
+                count: 3,
+                expectedValues: [1, 2, 3, 4, 5],
+            });
+
+            testAddRange({
+                description: "Add range overlapping the middle.",
+                initialValues: [1, 3, 5],
+                startIndex: 2,
+                count: 3,
+                expectedValues: [1, 2, 3, 4, 5],
+            });
+
+            testAddRange({
+                description: "Add range overlapping the beginning.",
+                initialValues: [3, 4, 5],
+                startIndex: 1,
+                count: 3,
+                expectedValues: [1, 2, 3, 4, 5],
+            });
+
+            testAddRange({
+                description: "Add range overlapping the end.",
+                initialValues: [1, 2, 3],
+                startIndex: 3,
+                count: 3,
+                expectedValues: [1, 2, 3, 4, 5],
+            });
+
+            return true;
+        }
+    });
+
+    suite.addTestCase({
+        name: "IndexSet.prototype.deleteRange",
+        test() {
+            function testDeleteRange({description, initialValues, startIndex, count, expectedValues}) {
+                let indexSet = createIndexSet(initialValues || []);
+
+                InspectorTest.log(description);
+                indexSet.deleteRange(startIndex, count);
+                InspectorTest.expectShallowEqual(Array.from(indexSet), expectedValues, `Should be [${expectedValues}] after removing [${rangeToArray(startIndex, count)}].`);
+                InspectorTest.log("");
+            }
+
+            testDeleteRange({
+                description: "Remove range from an empty IndexSet.",
+                initialValues: [],
+                startIndex: 1,
+                count: 3,
+                expectedValues: [],
+            });
+
+            testDeleteRange({
+                description: "Remove range before the beginning.",
+                initialValues: [10, 11, 12],
+                startIndex: 0,
+                count: 3,
+                expectedValues: [10, 11, 12],
+            });
+
+            testDeleteRange({
+                description: "Remove range after the end.",
+                initialValues: [0, 1, 2],
+                startIndex: 10,
+                count: 3,
+                expectedValues: [0, 1, 2],
+            });
+
+            testDeleteRange({
+                description: "Remove range in the middle.",
+                initialValues: [0, 1, 2, 3],
+                startIndex: 1,
+                count: 2,
+                expectedValues: [0, 3],
+            });
+
+            testDeleteRange({
+                description: "Remove range overlapping the middle.",
+                initialValues: [1, 3, 5],
+                startIndex: 2,
+                count: 3,
+                expectedValues: [1, 5],
+            });
+
+            testDeleteRange({
+                description: "Remove range overlapping the beginning.",
+                initialValues: [1, 2, 3],
+                startIndex: 0,
+                count: 3,
+                expectedValues: [3],
+            });
+
+            testDeleteRange({
+                description: "Remove range overlapping the end.",
+                initialValues: [1, 2, 3],
+                startIndex: 2,
+                count: 3,
+                expectedValues: [1],
+            });
+
+            return true;
+        }
+    });
+
+    suite.addTestCase({
+        name: "IndexSet.prototype.equals",
+        test() {
+            let original = new WI.IndexSet([1, 2, 3]);
+            let copied = original.copy();
+            InspectorTest.expectThat(original.equals(original), "Should trivially equal itself.");
+            InspectorTest.expectThat(original.equals(copied), "Copy and original should be equal.");
+
+            copied.delete(1);
+            InspectorTest.expectFalse(original.equals(copied), "Modified copy and original should not be equal.");
+
+            return true;
+        }
+    });
+
+    suite.addTestCase({
+        name: "IndexSet.prototype.difference",
+        test() {
+            function testDifference({values1, values2, expectedDifference}) {
+                let indexSet1 = new WI.IndexSet(values1);
+                let indexSet2 = new WI.IndexSet(values2);
+
+                InspectorTest.log(`Given an IndexSet with values [${values1}], and another IndexSet with values [${values2}]:`);
+
+                let difference = indexSet1.difference(indexSet2);
+                InspectorTest.expectShallowEqual(Array.from(difference), expectedDifference, `Difference between the first and second IndexSet should be [${expectedDifference}].`);
+                InspectorTest.log("");
+            }
+
+            testDifference({
+                values1: [],
+                values2: [],
+                expectedDifference: [],
+            });
+
+            testDifference({
+                values1: [1, 2, 3],
+                values2: [],
+                expectedDifference: [1, 2, 3],
+            });
+
+            testDifference({
+                values1: [],
+                values2: [1, 2, 3],
+                expectedDifference: [],
+            });
+
+            testDifference({
+                values1: [1, 2, 3],
+                values2: [2, 3, 4],
+                expectedDifference: [1],
+            });
+
+            return true;
+        }
+    });
+
     suite.runTestCasesAndFinish();
 }
 </script>
index f715ebf..7d849a9 100644 (file)
@@ -1,3 +1,50 @@
+2018-11-12  Matt Baker  <mattbaker@apple.com>
+
+        Web Inspector: Table should support shift-extending the row selection
+        https://bugs.webkit.org/show_bug.cgi?id=189718
+        <rdar://problem/44577942>
+
+        Reviewed by Devin Rousso.
+
+        Allow the table selection to be extended by shift-clicking a row, or by
+        holding shift and pressing either the up or down arrow key. If both command
+        and shift are pressed, shift is ignored. The selection behavior is modeled
+        after AppKit's NSTableView.
+
+        * UserInterface/Base/IndexSet.js:
+        (WI.IndexSet.prototype.addRange):
+        (WI.IndexSet.prototype.deleteRange):
+        (WI.IndexSet.prototype.equals):
+        (WI.IndexSet.prototype.difference):
+
+        * UserInterface/Views/Table.js:
+        (WI.Table):
+        (WI.Table.prototype.set allowsMultipleSelection):
+        (WI.Table.prototype.reloadData):
+        (WI.Table.prototype.selectRow):
+        (WI.Table.prototype.deselectRow):
+        (WI.Table.prototype._handleKeyDown):
+        Holding shift and pressing either the up or down arrow key extends the
+        selection to the next unselected row adjacent to the anchor row, or causes
+        the anchor row to be deselected, decreasing the selection. The table chooses
+        the action to take based on the direction of movement (up or down), and
+        the currently selected rows.
+
+        (WI.Table.prototype._selectRowsFromArrowKey):
+        (WI.Table.prototype._handleMouseDown.normalizeRange):
+        (WI.Table.prototype._handleMouseDown):
+        Clicking a row while holding down shift extends the selection to include
+        the rows between the anchor row (exclusive) and clicked row (inclusive).
+        The anchor row is equal to the value of `_selectedRowIndex` prior to
+        clicking a new row.
+
+        (WI.Table.prototype._deselectAllAndSelect):
+        (WI.Table.prototype._removeRows):
+        (WI.Table.prototype._toggleSelectedRowStyle):
+        (WI.Table.prototype._updateSelectedRows):
+        Helper method for updating the selection to the specified rows, and updating
+        DOM styles for rows that are added to or removed from the selection.
+
 2018-11-12  Nikita Vasilyev  <nvasilyev@apple.com>
 
         Web Inspector: Styles: inline swatches don't work when Multiple Properties Selection is enabled
index c8f8461..960432c 100644 (file)
@@ -91,6 +91,62 @@ WI.IndexSet = class IndexSet
         return this._indexes[index] === value;
     }
 
+    addRange(startIndex, count)
+    {
+        if (!this._validateIndex(startIndex))
+            return;
+
+        console.assert(count > 0);
+        if (count <= 0)
+            return;
+
+        if (count === 1) {
+            this.add(startIndex);
+            return;
+        }
+
+        let range = new Array(count);
+        for (let i = 0; i < count; ++i)
+            range[i] = startIndex + i;
+
+        if (!this.size || (this.firstIndex >= range[0] && this.lastIndex <= range.lastValue)) {
+            this._indexes = range;
+            return;
+        }
+
+        let start = this._indexes.lowerBound(startIndex);
+        let numberToDelete = this._indexes.upperBound(range.lastValue) - start;
+        this._indexes.splice(start, numberToDelete, ...range);
+    }
+
+    deleteRange(startIndex, count)
+    {
+        if (!this._validateIndex(startIndex))
+            return;
+
+        console.assert(count > 0);
+        if (count <= 0)
+            return;
+
+        if (!this.size)
+            return;
+
+        if (count === 1) {
+            this.delete(startIndex);
+            return;
+        }
+
+        let lastIndex = startIndex + count - 1;
+        if (this.firstIndex >= startIndex && this.lastIndex <= lastIndex) {
+            this.clear();
+            return;
+        }
+
+        let start = this._indexes.lowerBound(startIndex);
+        let numberToDelete = this._indexes.upperBound(lastIndex) - start;
+        this._indexes.splice(start, numberToDelete);
+    }
+
     clear()
     {
         this._indexes = [];
@@ -103,6 +159,30 @@ WI.IndexSet = class IndexSet
         return indexSet;
     }
 
+    equals(indexSet)
+    {
+        console.assert(indexSet instanceof WI.IndexSet);
+        if (!(indexSet instanceof WI.IndexSet))
+            return false;
+
+        if (indexSet === this)
+            return true;
+
+        return Array.shallowEqual(this._indexes, indexSet._indexes);
+    }
+
+    difference(indexSet)
+    {
+        console.assert(indexSet instanceof WI.IndexSet);
+
+        if (indexSet === this)
+            return new WI.IndexSet;
+
+        let result = new WI.IndexSet;
+        result._indexes = this._indexes.filter((value) => !indexSet.has(value));
+        return result;
+    }
+
     indexGreaterThan(value)
     {
         const following = true;
index f753d9f..2f024f0 100644 (file)
@@ -87,6 +87,7 @@ WI.Table = class Table extends WI.View
         this._columnWidths = null; // Calculated in _resizeColumnsAndFiller.
         this._fillerHeight = 0; // Calculated in _resizeColumnsAndFiller.
 
+        this._shiftAnchorIndex = NaN;
         this._selectedRowIndex = NaN;
         this._allowsMultipleSelection = false;
         this._selectedRows = new WI.IndexSet;
@@ -228,11 +229,14 @@ WI.Table = class Table extends WI.View
             return;
 
         this._allowsMultipleSelection = flag;
+        if (this._allowsMultipleSelection)
+            return;
 
-        let numberOfSelectedRows = this._selectedRows.size;
-        this._selectedRows.clear();
-        if (numberOfSelectedRows > 1)
+        if (this._selectedRows.size > 1) {
+            console.assert(this._selectedRowIndex >= 0);
+            this._selectedRows = new WI.IndexSet([this._selectedRowIndex]);
             this._notifySelectionDidChange();
+        }
     }
 
     isRowSelected(rowIndex)
@@ -252,6 +256,7 @@ WI.Table = class Table extends WI.View
     {
         this._cachedRows.clear();
 
+        this._shiftAnchorIndex = NaN;
         this._selectedRowIndex = NaN;
         this._selectedRows.clear();
 
@@ -337,12 +342,11 @@ WI.Table = class Table extends WI.View
             this.deselectAll();
         }
 
+        this._shiftAnchorIndex = NaN;
         this._selectedRowIndex = rowIndex;
         this._selectedRows.add(rowIndex);
 
-        let newSelectedRow = this._cachedRows.get(this._selectedRowIndex);
-        if (newSelectedRow)
-            newSelectedRow.classList.add("selected");
+        this._toggleSelectedRowStyle([this._selectedRowIndex], true);
 
         this._notifySelectionDidChange();
     }
@@ -354,12 +358,13 @@ WI.Table = class Table extends WI.View
         if (!this.isRowSelected(rowIndex))
             return;
 
-        let oldSelectedRow = this._cachedRows.get(rowIndex);
-        if (oldSelectedRow)
-            oldSelectedRow.classList.remove("selected");
+        this._toggleSelectedRowStyle([rowIndex], false);
 
         this._selectedRows.delete(rowIndex);
 
+        if (this._shiftAnchorIndex === rowIndex)
+            this._shiftAnchorIndex = NaN;
+
         if (this._selectedRowIndex === rowIndex) {
             this._selectedRowIndex = NaN;
             if (this._selectedRows.size) {
@@ -1273,26 +1278,17 @@ WI.Table = class Table extends WI.View
 
     _handleKeyDown(event)
     {
-        if (!this._isRowVisible(this._selectedRowIndex))
+        if (!this.numberOfRows)
             return;
 
-        if (event.shiftKey || event.metaKey || event.ctrlKey)
+        if (!this._isRowVisible(this._selectedRowIndex))
             return;
 
-        let numberOfRows = this.numberOfRows;
-        let rowToSelect = NaN;
-
-        if (event.keyIdentifier === "Up") {
-            if (this._selectedRowIndex > 0)
-                rowToSelect = this._selectedRowIndex - 1;
-        } else if (event.keyIdentifier === "Down") {
-            let numberOfRows = this._dataSource.tableNumberOfRows(this);
-            if (this._selectedRowIndex < (numberOfRows - 1))
-                rowToSelect = this._selectedRowIndex + 1;
-        }
+        if (event.metaKey || event.ctrlKey)
+            return;
 
-        if (!isNaN(rowToSelect)) {
-            this.selectRow(rowToSelect);
+        if (event.keyIdentifier === "Up" || event.keyIdentifier === "Down") {
+            this._selectRowsFromArrowKey(event.keyIdentifier === "Up", event.shiftKey);
 
             let row = this._cachedRows.get(this._selectedRowIndex);
             console.assert(row, "Moving up or down by one should always find a cached row since it is within the overflow bounds.");
@@ -1307,6 +1303,42 @@ WI.Table = class Table extends WI.View
         }
     }
 
+    _selectRowsFromArrowKey(goingUp, shiftKey)
+    {
+        let rowIncrement = goingUp ? -1 : 1;
+        let rowIndex = this._selectedRowIndex + rowIncrement;
+        if (rowIndex < 0 || rowIndex >= this.numberOfRows)
+            return;
+
+        let extendSelection = shiftKey && this._allowsMultipleSelection;
+
+        if (!extendSelection || !this.isRowSelected(rowIndex)) {
+            this.selectRow(rowIndex, extendSelection);
+            return;
+        }
+
+        // Since the row in the direction of movement is selected, we are either
+        // extending the selection into the row, or deselecting. Determine which
+        // by checking whether the row opposite the anchor row is selected.
+        let priorRowIndex = this._selectedRowIndex - rowIncrement;
+        if (!this.isRowSelected(priorRowIndex)) {
+            this.deselectRow(this._selectedRowIndex);
+            return;
+        }
+
+        // The selection is being extended into the row; make it the new
+        // anchor row then continue searching in the direction of movement
+        // for an unselected row to select.
+        for (; rowIndex >= 0 && rowIndex < this.numberOfRows; rowIndex += rowIncrement) {
+            if (!this.isRowSelected(rowIndex)) {
+                this.selectRow(rowIndex, extendSelection);
+                break;
+            }
+
+            this._selectedRowIndex = rowIndex;
+        }
+    }
+
     _handleMouseDown(event)
     {
         if (event.button !== 0 || event.ctrlKey)
@@ -1320,23 +1352,70 @@ WI.Table = class Table extends WI.View
         if (row === this._fillerRow)
             return;
 
-        let columnIndex = Array.from(row.children).indexOf(cell);
-        let column = this._visibleColumns[columnIndex];
         let rowIndex = row.__index;
+        let isRowSelected = this.isRowSelected(rowIndex);
+
+        // Before checking if multiple selection is allowed, check if clicking the
+        // row would cause it to be selected, and whether it is allowed by the delegate.
+        if (!isRowSelected && this._delegate.tableShouldSelectRow) {
+            let columnIndex = Array.from(row.children).indexOf(cell);
+            let column = this._visibleColumns[columnIndex];
+            if (!this._delegate.tableShouldSelectRow(this, cell, column, rowIndex))
+                return;
+        }
 
-        if (this.isRowSelected(rowIndex)) {
-            if (event.metaKey)
-                this.deselectRow(rowIndex)
+        // Command (meta) key takes precedence over shift whether or not multiple
+        // selection is enabled, so handle it first.
+        if (event.metaKey) {
+            if (isRowSelected)
+                this.deselectRow(rowIndex);
             else
-                this._deselectAllAndSelect(rowIndex);
+                this.selectRow(rowIndex, this._allowsMultipleSelection);
+            return;
+        }
+
+        let shiftExtendSelection = this._allowsMultipleSelection && event.shiftKey;
+        if (!shiftExtendSelection) {
+            this.selectRow(rowIndex);
             return;
         }
 
-        if (this._delegate.tableShouldSelectRow && !this._delegate.tableShouldSelectRow(this, cell, column, rowIndex))
+        let newSelectedRows = this._selectedRows.copy();
+
+        // Shift-clicking when nothing is selected should cause the first row
+        // through the clicked row to be selected.
+        if (!newSelectedRows.size) {
+            this._shiftAnchorIndex = 0;
+            this._selectedRowIndex = rowIndex;
+            newSelectedRows.addRange(0, rowIndex + 1);
+            this._updateSelectedRows(newSelectedRows);
             return;
+        }
+
+        if (isNaN(this._shiftAnchorIndex))
+            this._shiftAnchorIndex = this._selectedRowIndex;
+
+        // Shift-clicking will add to or delete from the current selection, or
+        // pivot the selection around the anchor (a delete followed by an add).
+        // We could check for all three cases, and add or delete only those rows
+        // that are necessary, but it is simpler to throw out the previous shift-
+        // selected range and add the new range between the anchor and clicked row.
+
+        function normalizeRange(startIndex, endIndex) {
+            return startIndex > endIndex ? [endIndex, startIndex] : [startIndex, endIndex];
+        }
+
+        if (this._shiftAnchorIndex !== this._selectedRowIndex) {
+            let [startIndex, endIndex] = normalizeRange(this._shiftAnchorIndex, this._selectedRowIndex);
+            newSelectedRows.deleteRange(startIndex, endIndex - startIndex + 1);
+        }
+
+        let [startIndex, endIndex] = normalizeRange(this._shiftAnchorIndex, rowIndex);
+        newSelectedRows.addRange(startIndex, endIndex - startIndex + 1);
+
+        this._selectedRowIndex = rowIndex;
 
-        let extendSelection = this._allowsMultipleSelection && event.metaKey;
-        this.selectRow(rowIndex, extendSelection);
+        this._updateSelectedRows(newSelectedRows);
     }
 
     _handleContextMenu(event)
@@ -1423,20 +1502,15 @@ WI.Table = class Table extends WI.View
         if (this._selectedRows.size === 1 && this._selectedRows.firstIndex === rowIndex)
             return;
 
-        for (let selectedRowIndex of this._selectedRows) {
-            let oldSelectedRow = this._cachedRows.get(selectedRowIndex);
-            if (oldSelectedRow)
-                oldSelectedRow.classList.remove("selected");
-        }
+        this._toggleSelectedRowStyle(this._selectedRows, false);
 
+        this._shiftAnchorIndex = NaN;
         this._selectedRowIndex = rowIndex;
         this._selectedRows.clear();
 
         if (!isNaN(rowIndex)) {
             this._selectedRows.add(rowIndex);
-            let newSelectedRow = this._cachedRows.get(rowIndex);
-            if (newSelectedRow)
-                newSelectedRow.classList.add("selected");
+            this._toggleSelectedRowStyle(this._selectedRows, true);
         }
 
         this._notifySelectionDidChange();
@@ -1463,6 +1537,8 @@ WI.Table = class Table extends WI.View
             }
         };
 
+        if (rowIndexes.has(this._shiftAnchorIndex))
+            this._shiftAnchorIndex = NaN;
         if (rowIndexes.has(this._selectedRowIndex))
             this._selectedRowIndex = NaN;
 
@@ -1506,6 +1582,33 @@ WI.Table = class Table extends WI.View
         if (this._delegate.tableSelectionDidChange)
             this._delegate.tableSelectionDidChange(this);
     }
+
+    _toggleSelectedRowStyle(rowIndexes, flag)
+    {
+        for (let index of rowIndexes) {
+            let row = this._cachedRows.get(index);
+            if (row)
+                row.classList.toggle("selected", flag);
+        }
+    }
+
+    _updateSelectedRows(rowIndexes)
+    {
+        if (this._selectedRows.equals(rowIndexes))
+            return;
+
+        let deselectedRows = this._selectedRows.difference(rowIndexes);
+        if (deselectedRows.size)
+            this._toggleSelectedRowStyle(deselectedRows, false);
+
+        let selectedRows = rowIndexes.difference(this._selectedRows);
+        if (selectedRows.size)
+            this._toggleSelectedRowStyle(selectedRows, true);
+
+        this._selectedRows = rowIndexes;
+
+        this._notifySelectionDidChange();
+    }
 };
 
 WI.Table.SortOrder = {