Title: [238121] trunk
Revision
238121
Author
mattba...@apple.com
Date
2018-11-12 21:13:02 -0800 (Mon, 12 Nov 2018)

Log Message

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.

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`.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (238120 => 238121)


--- trunk/LayoutTests/ChangeLog	2018-11-13 04:43:41 UTC (rev 238120)
+++ trunk/LayoutTests/ChangeLog	2018-11-13 05:13:02 UTC (rev 238121)
@@ -1,3 +1,15 @@
+2018-11-12  Matt Baker  <mattba...@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  <za...@apple.com>
 
         Do not collapse the soon-to-be-parent anon block when we shuffle around the marker item renderer.

Modified: trunk/LayoutTests/inspector/unit-tests/index-set-expected.txt (238120 => 238121)


--- trunk/LayoutTests/inspector/unit-tests/index-set-expected.txt	2018-11-13 04:43:41 UTC (rev 238120)
+++ trunk/LayoutTests/inspector/unit-tests/index-set-expected.txt	2018-11-13 05:13:02 UTC (rev 238121)
@@ -72,3 +72,82 @@
 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].
+
+

Modified: trunk/LayoutTests/inspector/unit-tests/index-set.html (238120 => 238121)


--- trunk/LayoutTests/inspector/unit-tests/index-set.html	2018-11-13 04:43:41 UTC (rev 238120)
+++ trunk/LayoutTests/inspector/unit-tests/index-set.html	2018-11-13 05:13:02 UTC (rev 238121)
@@ -15,6 +15,13 @@
         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 @@
         }
     });
 
+    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>

Modified: trunk/Source/WebInspectorUI/ChangeLog (238120 => 238121)


--- trunk/Source/WebInspectorUI/ChangeLog	2018-11-13 04:43:41 UTC (rev 238120)
+++ trunk/Source/WebInspectorUI/ChangeLog	2018-11-13 05:13:02 UTC (rev 238121)
@@ -1,3 +1,50 @@
+2018-11-12  Matt Baker  <mattba...@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  <nvasil...@apple.com>
 
         Web Inspector: Styles: inline swatches don't work when Multiple Properties Selection is enabled

Modified: trunk/Source/WebInspectorUI/UserInterface/Base/IndexSet.js (238120 => 238121)


--- trunk/Source/WebInspectorUI/UserInterface/Base/IndexSet.js	2018-11-13 04:43:41 UTC (rev 238120)
+++ trunk/Source/WebInspectorUI/UserInterface/Base/IndexSet.js	2018-11-13 05:13:02 UTC (rev 238121)
@@ -91,6 +91,62 @@
         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 @@
         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;

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/Table.js (238120 => 238121)


--- trunk/Source/WebInspectorUI/UserInterface/Views/Table.js	2018-11-13 04:43:41 UTC (rev 238120)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/Table.js	2018-11-13 05:13:02 UTC (rev 238121)
@@ -87,6 +87,7 @@
         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 @@
             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 @@
     {
         this._cachedRows.clear();
 
+        this._shiftAnchorIndex = NaN;
         this._selectedRowIndex = NaN;
         this._selectedRows.clear();
 
@@ -337,12 +342,11 @@
             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 @@
         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,27 +1278,18 @@
 
     _handleKeyDown(event)
     {
+        if (!this.numberOfRows)
+            return;
+
         if (!this._isRowVisible(this._selectedRowIndex))
             return;
 
-        if (event.shiftKey || event.metaKey || event.ctrlKey)
+        if (event.metaKey || event.ctrlKey)
             return;
 
-        let numberOfRows = this.numberOfRows;
-        let rowToSelect = NaN;
+        if (event.keyIdentifier === "Up" || event.keyIdentifier === "Down") {
+            this._selectRowsFromArrowKey(event.keyIdentifier === "Up", event.shiftKey);
 
-        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 (!isNaN(rowToSelect)) {
-            this.selectRow(rowToSelect);
-
             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.");
             row.scrollIntoViewIfNeeded(false);
@@ -1307,6 +1303,42 @@
         }
     }
 
+    _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 @@
         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);
 
-        if (this.isRowSelected(rowIndex)) {
-            if (event.metaKey)
-                this.deselectRow(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;
+        }
+
+        // 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;
         }
 
-        if (this._delegate.tableShouldSelectRow && !this._delegate.tableShouldSelectRow(this, cell, column, rowIndex))
+        let shiftExtendSelection = this._allowsMultipleSelection && event.shiftKey;
+        if (!shiftExtendSelection) {
+            this.selectRow(rowIndex);
             return;
+        }
 
-        let extendSelection = this._allowsMultipleSelection && event.metaKey;
-        this.selectRow(rowIndex, extendSelection);
+        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;
+
+        this._updateSelectedRows(newSelectedRows);
     }
 
     _handleContextMenu(event)
@@ -1423,20 +1502,15 @@
         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 @@
             }
         };
 
+        if (rowIndexes.has(this._shiftAnchorIndex))
+            this._shiftAnchorIndex = NaN;
         if (rowIndexes.has(this._selectedRowIndex))
             this._selectedRowIndex = NaN;
 
@@ -1506,6 +1582,33 @@
         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 = {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to