Web Inspector: Table should support multiple selection and Cmd-click behavior
authormattbaker@apple.com <mattbaker@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 4 Oct 2018 21:54:14 +0000 (21:54 +0000)
committermattbaker@apple.com <mattbaker@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 4 Oct 2018 21:54:14 +0000 (21:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=189705
<rdar://problem/44571170>

Reviewed by Devin Rousso.

Source/WebInspectorUI:

Add multiple row selection to Table, with new methods for programmatic
selection (deselectRow, deselectAll), and Command-click support for
selecting/deselecting Table rows.

* UserInterface/Base/IndexSet.js: Added.
(WI.IndexSet):
(WI.IndexSet.prototype.get size):
(WI.IndexSet.prototype.get firstIndex):
(WI.IndexSet.prototype.get lastIndex):
(WI.IndexSet.prototype.add):
(WI.IndexSet.prototype.delete):
(WI.IndexSet.prototype.has):
(WI.IndexSet.prototype.clear):
(WI.IndexSet.prototype.indexGreaterThan):
(WI.IndexSet.prototype.indexLessThan):
(WI.IndexSet.prototype.Symbol.iterator):
(WI.IndexSet.prototype._indexClosestTo):
(WI.IndexSet.prototype._validateIndex):
Helper container for managing an ordered sequence of unique positive
integers, with set semantics, backed by a sorted array. Used by Table,
and eventually by TreeOutline.

* UserInterface/Main.html:
* UserInterface/Test.html:
* UserInterface/Test/Test.js:
New files and stubs to make Table layout tests possible.

* UserInterface/Views/NetworkTableContentView.js:
(WI.NetworkTableContentView.prototype.reset):
(WI.NetworkTableContentView.prototype.showRepresentedObject):
(WI.NetworkTableContentView.prototype.networkResourceDetailViewClose):
(WI.NetworkTableContentView.prototype.tableSelectionDidChange):
(WI.NetworkTableContentView.prototype._restoreSelectedRow):
(WI.NetworkTableContentView.prototype.tableSelectedRowChanged): Deleted.
Replace uses of `clearSelectedRow` with `deselectAll`, and updated
selection changed delegate.

* UserInterface/Views/Table.css:
(.table > .data-container > .data-list > li):
(.table > .data-container > .data-list > li.selected):
(@media (prefers-dark-interface)):
(.table,): Deleted.
Removed styles that are no longer needed after https://webkit.org/b/189766,
and provide a visual separation between adjacent selected rows.

* UserInterface/Views/Table.js:
(WI.Table):
(WI.Table.prototype.get selectedRows):
(WI.Table.prototype.get allowsMultipleSelection):
(WI.Table.prototype.set allowsMultipleSelection):
(WI.Table.prototype.reloadData):
(WI.Table.prototype.selectRow):
(WI.Table.prototype.deselectRow):
(WI.Table.prototype.deselectAll):
(WI.Table.prototype._getOrCreateRow):
(WI.Table.prototype._handleMouseDown):
(WI.Table.prototype._deselectAllAndSelect):
(WI.Table.prototype._isRowSelected):
(WI.Table.prototype._notifySelectionDidChange):
(WI.Table.prototype.clearSelectedRow): Deleted.
Table now tracks selected rows using an IndexSet. selectRow accepts an
optional parameter, `extendSelection`, for adding rows to the selection.
_selectedRowIndex is now used to track the most recently selected row.
This will be the only selected row unless multiple selection is enabled,
in which case it is the row that has the "focus", for purposes of selecting
a new row using the up or down arrow keys.

LayoutTests:

* inspector/table/resources/table-utilities.js: Added.
(TestPage.registerInitializer.InspectorTest.TableDataSource):
(TestPage.registerInitializer.InspectorTest.TableDataSource.prototype.get items):
(TestPage.registerInitializer.InspectorTest.TableDataSource.prototype.tableNumberOfRows):
(TestPage.registerInitializer.InspectorTest.TableDelegate):
(TestPage.registerInitializer.InspectorTest.TableDelegate.prototype.tableSelectionDidChange):
(TestPage.registerInitializer.InspectorTest.TableDelegate.prototype.tablePopulateCell):
(TestPage.registerInitializer.InspectorTest.createTable):
(TestPage.registerInitializer):

* inspector/table/table-selection-expected.txt: Added.
* inspector/table/table-selection.html: Added.
* inspector/unit-tests/index-set-expected.txt: Added.
* inspector/unit-tests/index-set.html: Added.

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

14 files changed:
LayoutTests/ChangeLog
LayoutTests/inspector/table/resources/table-utilities.js [new file with mode: 0644]
LayoutTests/inspector/table/table-selection-expected.txt [new file with mode: 0644]
LayoutTests/inspector/table/table-selection.html [new file with mode: 0644]
LayoutTests/inspector/unit-tests/index-set-expected.txt [new file with mode: 0644]
LayoutTests/inspector/unit-tests/index-set.html [new file with mode: 0644]
Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Base/IndexSet.js [new file with mode: 0644]
Source/WebInspectorUI/UserInterface/Main.html
Source/WebInspectorUI/UserInterface/Test.html
Source/WebInspectorUI/UserInterface/Test/Test.js
Source/WebInspectorUI/UserInterface/Views/NetworkTableContentView.js
Source/WebInspectorUI/UserInterface/Views/Table.css
Source/WebInspectorUI/UserInterface/Views/Table.js

index c7b9fb6..4e493bf 100644 (file)
@@ -1,3 +1,26 @@
+2018-10-04  Matt Baker  <mattbaker@apple.com>
+
+        Web Inspector: Table should support multiple selection and Cmd-click behavior
+        https://bugs.webkit.org/show_bug.cgi?id=189705
+        <rdar://problem/44571170>
+
+        Reviewed by Devin Rousso.
+
+        * inspector/table/resources/table-utilities.js: Added.
+        (TestPage.registerInitializer.InspectorTest.TableDataSource):
+        (TestPage.registerInitializer.InspectorTest.TableDataSource.prototype.get items):
+        (TestPage.registerInitializer.InspectorTest.TableDataSource.prototype.tableNumberOfRows):
+        (TestPage.registerInitializer.InspectorTest.TableDelegate):
+        (TestPage.registerInitializer.InspectorTest.TableDelegate.prototype.tableSelectionDidChange):
+        (TestPage.registerInitializer.InspectorTest.TableDelegate.prototype.tablePopulateCell):
+        (TestPage.registerInitializer.InspectorTest.createTable):
+        (TestPage.registerInitializer):
+
+        * inspector/table/table-selection-expected.txt: Added.
+        * inspector/table/table-selection.html: Added.
+        * inspector/unit-tests/index-set-expected.txt: Added.
+        * inspector/unit-tests/index-set.html: Added.
+
 2018-10-03  Ryosuke Niwa  <rniwa@webkit.org>
 
         MutationRecord doesn't keep JS wrappers of target, addedNodes, and removedNodes alive
diff --git a/LayoutTests/inspector/table/resources/table-utilities.js b/LayoutTests/inspector/table/resources/table-utilities.js
new file mode 100644 (file)
index 0000000..4861a35
--- /dev/null
@@ -0,0 +1,64 @@
+TestPage.registerInitializer(() => {
+    InspectorTest.TableDataSource = class TableDataSource
+    {
+        constructor(items)
+        {
+            this._items = items;
+        }
+
+        get items() { return this._items; }
+
+        // Table DataSource
+
+        tableNumberOfRows(table)
+        {
+            return this._items.length;
+        }
+    }
+
+    InspectorTest.TableDelegate = class TableDelegate
+    {
+        constructor(items)
+        {
+            this._items = items;
+        }
+
+        tableSelectionDidChange(table)
+        {
+            InspectorTest.pass("Table selection changed.");
+        }
+
+        tablePopulateCell(table, cell, column, rowIndex)
+        {
+            let item = this._items[rowIndex];
+            InspectorTest.assert(item, "Should have an item for row " + rowIndex);
+            InspectorTest.assert(item[column.identifier], "Should have data for column " + column.identifier);
+            cell.textContent = item[column.identifier];
+            return cell;
+        }
+    }
+
+    InspectorTest.createTable = function(delegate, dataSource) {
+        if (!dataSource) {
+            let items = [];
+            for (let i = 0; i < 10; ++i) {
+                items.push({
+                    index: i,
+                    name: "Row " + i,
+                });
+            }
+            dataSource = new InspectorTest.TableDataSource(items);
+        }
+
+        delegate = delegate || new InspectorTest.TableDelegate(dataSource.items);
+
+        const rowHeight = 20;
+        let table = new WI.Table("test", dataSource, delegate, rowHeight);
+        table.addColumn(new WI.TableColumn("index", WI.UIString("Index")));
+        table.addColumn(new WI.TableColumn("name", WI.UIString("Name")));
+
+        table.updateLayout();
+
+        return table;
+    }
+});
diff --git a/LayoutTests/inspector/table/table-selection-expected.txt b/LayoutTests/inspector/table/table-selection-expected.txt
new file mode 100644 (file)
index 0000000..63045f8
--- /dev/null
@@ -0,0 +1,68 @@
+Tests for WI.Table.
+
+
+== Running test suite: Table
+-- Running test case: Table.constructor
+PASS: selectedRow should be NaN.
+PASS: Should have no selected rows.
+PASS: allowsMultipleSelection should initially be false.
+
+-- Running test case: Table.SelectRow
+Selecting row 0.
+PASS: Table selection changed.
+PASS: selectedRow should be 0.
+PASS: selectedRows should be [0].
+Selecting row 1.
+PASS: Table selection changed.
+PASS: selectedRow should be 1.
+PASS: selectedRows should be [1].
+
+-- Running test case: Table.DeselectRow
+Selecting row 0.
+PASS: Table selection changed.
+PASS: selectedRow should be 0.
+Deselecting row 0.
+PASS: Table selection changed.
+PASS: selectedRows should not include 0.
+PASS: Should have no selected rows.
+
+-- Running test case: Table.AllowsMultipleSelection
+PASS: allowsMultipleSelection enabled.
+PASS: allowsMultipleSelection disabled.
+
+-- Running test case: Table.SelectMultipleRows.ExtendSelection
+PASS: allowsMultipleSelection enabled.
+Selecting row 0.
+PASS: Table selection changed.
+PASS: selectedRow should be 0.
+Selecting row 1.
+PASS: Table selection changed.
+PASS: selectedRow should be 1.
+Selecting row 9.
+PASS: Table selection changed.
+PASS: selectedRow should be 9.
+PASS: selectedRows should be [0, 1, 9].
+
+-- Running test case: Table.SelectMultipleRows.SelectTheSameRowTwice.ExtendSelection
+Selecting row 0.
+PASS: Table selection changed.
+PASS: selectedRow should be 0.
+Selecting row 1.
+PASS: Table selection changed.
+PASS: selectedRow should be 1.
+Selecting row 1.
+PASS: selectedRow should be 1.
+PASS: selectedRows should be [0, 1].
+
+-- Running test case: Table.SelectMultipleRows.SelectTheSameRowTwice.NoExtendSelection
+Selecting row 0.
+PASS: Table selection changed.
+PASS: selectedRow should be 0.
+Selecting row 1.
+PASS: Table selection changed.
+PASS: selectedRow should be 1.
+Selecting row 1.
+PASS: Table selection changed.
+PASS: selectedRow should be 1.
+PASS: selectedRows should be [1].
+
diff --git a/LayoutTests/inspector/table/table-selection.html b/LayoutTests/inspector/table/table-selection.html
new file mode 100644 (file)
index 0000000..32ae6c4
--- /dev/null
@@ -0,0 +1,146 @@
+<!doctype html>
+<html>
+<head>
+<script src="../../http/tests/inspector/resources/inspector-test.js"></script>
+<script src="resources/table-utilities.js"></script>
+<script>
+function test()
+{
+    InspectorTest.redirectRequestAnimationFrame();
+
+    let suite = InspectorTest.createSyncSuite("Table");
+
+    // Import names.
+    let {createTable, cleanupTable} = InspectorTest;
+
+    suite.addTestCase({
+        name: "Table.constructor",
+        test() {
+            let table = createTable();
+
+            InspectorTest.expectThat(isNaN(table.selectedRow), "selectedRow should be NaN.");
+            InspectorTest.expectEqual(table.selectedRows.length, 0, "Should have no selected rows.");
+            InspectorTest.expectFalse(table.allowsMultipleSelection, "allowsMultipleSelection should initially be false.");
+
+            return true;
+        }
+    });
+
+    function triggerSelectRow(table, rowIndex, extendSelection) {
+        InspectorTest.log(`Selecting row ${rowIndex}.`);
+        table.selectRow(rowIndex, extendSelection);
+        InspectorTest.expectEqual(table.selectedRow, rowIndex, `selectedRow should be ${rowIndex}.`);
+    }
+
+    function triggerDeselectRow(table, rowIndex) {
+        InspectorTest.log(`Deselecting row ${rowIndex}.`);
+        table.deselectRow(rowIndex);
+        InspectorTest.expectFalse(table.selectedRows.includes(rowIndex), `selectedRows should not include ${rowIndex}.`);
+    }
+
+    suite.addTestCase({
+        name: "Table.SelectRow",
+        description: "Select a row, then select another row causing the frist to become deselected.",
+        test() {
+            let table = createTable();
+
+            triggerSelectRow(table, 0);
+            InspectorTest.expectShallowEqual(table.selectedRows, [0], "selectedRows should be [0].");
+            triggerSelectRow(table, 1);
+            InspectorTest.expectShallowEqual(table.selectedRows, [1], "selectedRows should be [1].");
+
+            return true;
+        }
+    });
+
+    suite.addTestCase({
+        name: "Table.DeselectRow",
+        description: "Deselect the selected row.",
+        test() {
+            let table = createTable();
+
+            triggerSelectRow(table, 0);
+            triggerDeselectRow(table, 0);
+            InspectorTest.expectEqual(table.selectedRows.length, 0, "Should have no selected rows.");
+
+            return true;
+        }
+    });
+
+    suite.addTestCase({
+        name: "Table.AllowsMultipleSelection",
+        description: "Should be able to enable multiple selection.",
+        test() {
+            let table = createTable();
+
+            table.allowsMultipleSelection = true;
+            InspectorTest.expectThat(table.allowsMultipleSelection, "allowsMultipleSelection enabled.");
+            table.allowsMultipleSelection = false;
+            InspectorTest.expectFalse(table.allowsMultipleSelection, "allowsMultipleSelection disabled.");
+
+            return true;
+        }
+    });
+    suite.addTestCase({
+        name: "Table.SelectMultipleRows.ExtendSelection",
+        description: "Select multiple rows, extending the selection.",
+        test() {
+            let table = createTable();
+            table.allowsMultipleSelection = true;
+            InspectorTest.expectThat(table.allowsMultipleSelection, "allowsMultipleSelection enabled.");
+
+            const extendSelection = true;
+
+            triggerSelectRow(table, 0, extendSelection);
+            triggerSelectRow(table, 1, extendSelection);
+            triggerSelectRow(table, 9, extendSelection);
+            InspectorTest.expectShallowEqual(table.selectedRows, [0, 1, 9], "selectedRows should be [0, 1, 9].");
+
+            return true;
+        }
+    });
+
+    suite.addTestCase({
+        name: "Table.SelectMultipleRows.SelectTheSameRowTwice.ExtendSelection",
+        description: "Select an already selected row, and extend the selection.",
+        test() {
+            let table = createTable();
+            table.allowsMultipleSelection = true;
+
+            const extendSelection = true;
+
+            triggerSelectRow(table, 0, extendSelection);
+            triggerSelectRow(table, 1, extendSelection);
+            triggerSelectRow(table, 1, extendSelection);
+            InspectorTest.expectShallowEqual(table.selectedRows, [0, 1], "selectedRows should be [0, 1].");
+
+            return true;
+        }
+    });
+
+    suite.addTestCase({
+        name: "Table.SelectMultipleRows.SelectTheSameRowTwice.NoExtendSelection",
+        description: "Select an already selected row, and do not extend the selection.",
+        test() {
+            let table = createTable();
+            table.allowsMultipleSelection = true;
+
+            const extendSelection = true;
+
+            triggerSelectRow(table, 0, extendSelection);
+            triggerSelectRow(table, 1, extendSelection);
+            triggerSelectRow(table, 1);
+            InspectorTest.expectShallowEqual(table.selectedRows, [1], "selectedRows should be [1].");
+
+            return true;
+        }
+    });
+
+    suite.runTestCasesAndFinish();
+}
+</script>
+</head>
+<body onLoad="runTest()">
+    <p>Tests for WI.Table.</p>
+</body>
+</html>
diff --git a/LayoutTests/inspector/unit-tests/index-set-expected.txt b/LayoutTests/inspector/unit-tests/index-set-expected.txt
new file mode 100644 (file)
index 0000000..c3363cf
--- /dev/null
@@ -0,0 +1,70 @@
+Tests for WI.IndexSet.
+
+
+== Running test suite: IndexSet
+-- Running test case: IndexSet.constructor
+PASS: size should be zero.
+PASS: firstIndex should be NaN.
+PASS: lastIndex should be NaN.
+PASS: Should be [].
+
+-- Running test case: IndexSet.constructor array
+Initialize IndexSet with an array.
+PASS: size should be 5.
+PASS: firstIndex should be 0.
+PASS: lastIndex should be 100.
+PASS: Should be [0,1,5,50,100].
+Initialize IndexSet with an array containing duplicate indexes.
+PASS: size should be 5.
+PASS: Should be [0,1,5,50,100].
+
+-- Running test case: IndexSet.constructor invalid
+PASS: size should be zero.
+
+-- Running test case: IndexSet.prototype.clear
+PASS: size should be zero.
+PASS: firstIndex should be NaN.
+PASS: lastIndex should be NaN.
+PASS: Should be [].
+
+-- Running test case: IndexSet.prototype.add
+PASS: size should be 1.
+PASS: has should return true.
+
+-- Running test case: IndexSet.prototype.add duplicate
+PASS: size should be 1.
+
+-- Running test case: IndexSet.prototype.add invalid
+PASS: size should be zero.
+
+-- Running test case: IndexSet.prototype.delete
+Given an IndexSet with values [1,2,3]:
+PASS: delete 3 should succeed.
+PASS: has 3 should return false.
+
+-- Running test case: IndexSet.prototype.delete nonexistent
+Given an IndexSet with values [1,2,3]:
+PASS: delete 4 should fail.
+
+-- Running test case: IndexSet.prototype.delete invalid
+-- Running test case: IndexSet.prototype[Symbol.iterator]
+Given an IndexSet with values [20,1,10,2]:
+1
+2
+10
+20
+
+-- Running test case: IndexSet.prototype.indexGreaterThan
+Given an IndexSet with values [1,2]:
+PASS: Index greater than 0 should be 1.
+PASS: Index greater than 1 should be 2.
+PASS: Index greater than 2 should be NaN.
+PASS: Index greater than 3 should be NaN.
+
+-- Running test case: IndexSet.prototype.indexLessThan
+Given an IndexSet with values [1,2]:
+PASS: Index less than 0 should be NaN.
+PASS: Index less than 1 should be NaN.
+PASS: Index less than 2 should be 1.
+PASS: Index less than 3 should be 2.
+
diff --git a/LayoutTests/inspector/unit-tests/index-set.html b/LayoutTests/inspector/unit-tests/index-set.html
new file mode 100644 (file)
index 0000000..05a3db9
--- /dev/null
@@ -0,0 +1,204 @@
+<!doctype html>
+<html>
+<head>
+<script src="../../http/tests/inspector/resources/inspector-test.js"></script>
+<script>
+function test()
+{
+    let suite = InspectorTest.createSyncSuite("IndexSet");
+
+    function createIndexSet(values = []) {
+        if (!Array.isArray(values))
+            values = [values];
+        InspectorTest.log(`Given an IndexSet with values [${values}]:`);
+
+        return new WI.IndexSet(values);
+    }
+
+    suite.addTestCase({
+        name: "IndexSet.constructor",
+        test() {
+            let indexSet = new WI.IndexSet;
+            InspectorTest.expectEqual(indexSet.size, 0, "size should be zero.");
+            InspectorTest.expectThat(isNaN(indexSet.firstIndex), "firstIndex should be NaN.");
+            InspectorTest.expectThat(isNaN(indexSet.lastIndex), "lastIndex should be NaN.");
+            InspectorTest.expectShallowEqual(Array.from(indexSet), [], "Should be [].");
+
+            return true;
+        }
+    });
+
+    suite.addTestCase({
+        name: "IndexSet.constructor array",
+        test() {
+            const values = [5, 1, 0, 100, 50];
+            const sortedValues = values.slice().sort((a, b) => a - b);
+
+            InspectorTest.log("Initialize IndexSet with an array.");
+            {
+                let indexSet = new WI.IndexSet(values);
+                InspectorTest.expectEqual(indexSet.size, values.length, `size should be ${values.length}.`);
+                InspectorTest.expectEqual(indexSet.firstIndex, sortedValues[0], `firstIndex should be ${sortedValues[0]}.`);
+                InspectorTest.expectEqual(indexSet.lastIndex, sortedValues.lastValue, `lastIndex should be ${sortedValues.lastValue}.`);
+                InspectorTest.expectShallowEqual(Array.from(indexSet), sortedValues, `Should be [${sortedValues}].`);
+            }
+
+            InspectorTest.log("Initialize IndexSet with an array containing duplicate indexes.");
+            {
+                let indexSet = new WI.IndexSet(values.concat(values));
+                InspectorTest.expectEqual(indexSet.size, values.length, `size should be ${values.length}.`);
+                InspectorTest.expectShallowEqual(Array.from(indexSet), sortedValues, `Should be [${sortedValues}].`);
+            }
+
+            return true;
+        }
+    });
+
+    suite.addTestCase({
+        name: "IndexSet.constructor invalid",
+        test() {
+            let indexSet = new WI.IndexSet([-1, 1.5, "abc"]);
+            InspectorTest.expectEqual(indexSet.size, 0, "size should be zero.");
+            return true;
+        }
+    });
+
+    suite.addTestCase({
+        name: "IndexSet.prototype.clear",
+        test() {
+            let indexSet = new WI.IndexSet([1, 2, 3]);
+            indexSet.add(42);
+            indexSet.clear();
+            InspectorTest.expectEqual(indexSet.size, 0, "size should be zero.");
+            InspectorTest.expectThat(isNaN(indexSet.firstIndex), "firstIndex should be NaN.");
+            InspectorTest.expectThat(isNaN(indexSet.lastIndex), "lastIndex should be NaN.");
+            InspectorTest.expectShallowEqual(Array.from(indexSet), [], "Should be [].");
+
+            return true;
+        }
+    });
+
+    suite.addTestCase({
+        name: "IndexSet.prototype.add",
+        test() {
+            let indexSet = new WI.IndexSet;
+            indexSet.add(42);
+            InspectorTest.expectEqual(indexSet.size, 1, "size should be 1.");
+            InspectorTest.expectThat(indexSet.has(42), "has should return true.");
+
+            return true;
+        }
+    });
+
+    suite.addTestCase({
+        name: "IndexSet.prototype.add duplicate",
+        test() {
+            let indexSet = new WI.IndexSet;
+            indexSet.add(42);
+            indexSet.add(42);
+            InspectorTest.expectEqual(indexSet.size, 1, "size should be 1.");
+
+            return true;
+        }
+    });
+
+    suite.addTestCase({
+        name: "IndexSet.prototype.add invalid",
+        test() {
+            let indexSet = new WI.IndexSet;
+            indexSet.add(-1);
+            indexSet.add(1.5);
+            indexSet.add("abc");
+            InspectorTest.expectEqual(indexSet.size, 0, "size should be zero.");
+
+            return true;
+        }
+    });
+
+    suite.addTestCase({
+        name: "IndexSet.prototype.delete",
+        test() {
+            let indexSet = createIndexSet([1, 2, 3]);
+            const indexToDelete = indexSet.lastIndex;
+            let wasDeleted = indexSet.delete(indexToDelete);
+            InspectorTest.expectThat(wasDeleted, `delete ${indexToDelete} should succeed.`);
+            InspectorTest.expectFalse(indexSet.has(indexToDelete), `has ${indexToDelete} should return false.`);
+
+            return true;
+        }
+    });
+
+    suite.addTestCase({
+        name: "IndexSet.prototype.delete nonexistent",
+        test() {
+            let indexSet = createIndexSet([1, 2, 3]);
+            const indexToDelete = indexSet.lastIndex + 1;
+            let wasDeleted = indexSet.delete(indexToDelete);
+            InspectorTest.expectFalse(wasDeleted, `delete ${indexToDelete} should fail.`);
+
+            return true;
+        }
+    });
+
+    suite.addTestCase({
+        name: "IndexSet.prototype.delete invalid",
+        test() {
+            let indexSet = new WI.IndexSet;
+            indexSet.delete(-1);
+
+            return true;
+        }
+    });
+
+    suite.addTestCase({
+        name: "IndexSet.prototype[Symbol.iterator]",
+        test() {
+            let indexSet = createIndexSet([20, 1, 10, 2]);
+            for (let index of indexSet)
+                InspectorTest.log(index);
+
+            return true;
+        }
+    });
+
+    suite.addTestCase({
+        name: "IndexSet.prototype.indexGreaterThan",
+        test() {
+            let indexSet = createIndexSet([1, 2]);
+            const {firstIndex, lastIndex} = indexSet;
+            const indexBefore = firstIndex - 1;
+            const indexAfter = lastIndex + 1;
+            InspectorTest.expectEqual(indexSet.indexGreaterThan(indexBefore), firstIndex, `Index greater than ${indexBefore} should be ${firstIndex}.`);
+            InspectorTest.expectEqual(indexSet.indexGreaterThan(firstIndex), lastIndex, `Index greater than ${firstIndex} should be ${lastIndex}.`);
+            InspectorTest.expectThat(isNaN(indexSet.indexGreaterThan(lastIndex)), `Index greater than ${lastIndex} should be NaN.`);
+            InspectorTest.expectThat(isNaN(indexSet.indexGreaterThan(indexAfter)), `Index greater than ${indexAfter} should be NaN.`);
+
+            return true;
+        }
+    });
+
+    suite.addTestCase({
+        name: "IndexSet.prototype.indexLessThan",
+        test() {
+            let indexSet = createIndexSet([1, 2]);
+            const {firstIndex, lastIndex} = indexSet;
+            const indexBefore = firstIndex - 1;
+            const indexAfter = lastIndex + 1;
+
+            InspectorTest.expectThat(isNaN(indexSet.indexLessThan(indexBefore)), `Index less than ${indexBefore} should be NaN.`);
+            InspectorTest.expectThat(isNaN(indexSet.indexLessThan(firstIndex)), `Index less than ${firstIndex} should be NaN.`);
+            InspectorTest.expectEqual(indexSet.indexLessThan(lastIndex), firstIndex, `Index less than ${lastIndex} should be ${firstIndex}.`);
+            InspectorTest.expectEqual(indexSet.indexLessThan(indexAfter), lastIndex, `Index less than ${indexAfter} should be ${lastIndex}.`);
+
+            return true;
+        }
+    });
+
+    suite.runTestCasesAndFinish();
+}
+</script>
+</head>
+<body onLoad="runTest()">
+    <p>Tests for WI.IndexSet.</p>
+</body>
+</html>
index 23b1829..1220a07 100644 (file)
@@ -1,3 +1,78 @@
+2018-10-04  Matt Baker  <mattbaker@apple.com>
+
+        Web Inspector: Table should support multiple selection and Cmd-click behavior
+        https://bugs.webkit.org/show_bug.cgi?id=189705
+        <rdar://problem/44571170>
+
+        Reviewed by Devin Rousso.
+
+        Add multiple row selection to Table, with new methods for programmatic
+        selection (deselectRow, deselectAll), and Command-click support for
+        selecting/deselecting Table rows.
+
+        * UserInterface/Base/IndexSet.js: Added.
+        (WI.IndexSet):
+        (WI.IndexSet.prototype.get size):
+        (WI.IndexSet.prototype.get firstIndex):
+        (WI.IndexSet.prototype.get lastIndex):
+        (WI.IndexSet.prototype.add):
+        (WI.IndexSet.prototype.delete):
+        (WI.IndexSet.prototype.has):
+        (WI.IndexSet.prototype.clear):
+        (WI.IndexSet.prototype.indexGreaterThan):
+        (WI.IndexSet.prototype.indexLessThan):
+        (WI.IndexSet.prototype.Symbol.iterator):
+        (WI.IndexSet.prototype._indexClosestTo):
+        (WI.IndexSet.prototype._validateIndex):
+        Helper container for managing an ordered sequence of unique positive
+        integers, with set semantics, backed by a sorted array. Used by Table,
+        and eventually by TreeOutline.
+
+        * UserInterface/Main.html:
+        * UserInterface/Test.html:
+        * UserInterface/Test/Test.js:
+        New files and stubs to make Table layout tests possible.
+
+        * UserInterface/Views/NetworkTableContentView.js:
+        (WI.NetworkTableContentView.prototype.reset):
+        (WI.NetworkTableContentView.prototype.showRepresentedObject):
+        (WI.NetworkTableContentView.prototype.networkResourceDetailViewClose):
+        (WI.NetworkTableContentView.prototype.tableSelectionDidChange):
+        (WI.NetworkTableContentView.prototype._restoreSelectedRow):
+        (WI.NetworkTableContentView.prototype.tableSelectedRowChanged): Deleted.
+        Replace uses of `clearSelectedRow` with `deselectAll`, and updated
+        selection changed delegate.
+
+        * UserInterface/Views/Table.css:
+        (.table > .data-container > .data-list > li):
+        (.table > .data-container > .data-list > li.selected):
+        (@media (prefers-dark-interface)):
+        (.table,): Deleted.
+        Removed styles that are no longer needed after https://webkit.org/b/189766,
+        and provide a visual separation between adjacent selected rows.
+
+        * UserInterface/Views/Table.js:
+        (WI.Table):
+        (WI.Table.prototype.get selectedRows):
+        (WI.Table.prototype.get allowsMultipleSelection):
+        (WI.Table.prototype.set allowsMultipleSelection):
+        (WI.Table.prototype.reloadData):
+        (WI.Table.prototype.selectRow):
+        (WI.Table.prototype.deselectRow):
+        (WI.Table.prototype.deselectAll):
+        (WI.Table.prototype._getOrCreateRow):
+        (WI.Table.prototype._handleMouseDown):
+        (WI.Table.prototype._deselectAllAndSelect):
+        (WI.Table.prototype._isRowSelected):
+        (WI.Table.prototype._notifySelectionDidChange):
+        (WI.Table.prototype.clearSelectedRow): Deleted.
+        Table now tracks selected rows using an IndexSet. selectRow accepts an
+        optional parameter, `extendSelection`, for adding rows to the selection.
+        _selectedRowIndex is now used to track the most recently selected row.
+        This will be the only selected row unless multiple selection is enabled,
+        in which case it is the row that has the "focus", for purposes of selecting
+        a new row using the up or down arrow keys.
+
 2018-10-04  Devin Rousso  <drousso@apple.com>
 
         Web Inspector: REGRESSION(r236540): Uncaught Exception: TypeError: pauseReasonBreakpointTreeElement.removeStatusImage is not a function.
diff --git a/Source/WebInspectorUI/UserInterface/Base/IndexSet.js b/Source/WebInspectorUI/UserInterface/Base/IndexSet.js
new file mode 100644 (file)
index 0000000..f48cd71
--- /dev/null
@@ -0,0 +1,149 @@
+/*
+ * Copyright (C) 2018 Apple Inc. All Rights Reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+WI.IndexSet = class IndexSet
+{
+    constructor(values)
+    {
+        console.assert(!values || Array.isArray(values));
+
+        this._indexes = [];
+
+        if (values) {
+            for (let value of values.slice().sort((a, b) => a - b)) {
+                if (value === this._indexes.lastValue)
+                    continue;
+                if (this._validateIndex(value))
+                    this._indexes.push(value);
+            }
+        }
+    }
+
+    // Public
+
+    get size() { return this._indexes.length; }
+
+    get firstIndex()
+    {
+        return this._indexes.length ? this._indexes[0] : NaN;
+    }
+
+    get lastIndex()
+    {
+        return this._indexes.length ? this._indexes.lastValue : NaN;
+    }
+
+    add(value)
+    {
+        if (!this._validateIndex(value))
+            return;
+
+        let index = this._indexes.lowerBound(value);
+        if (this._indexes[index] === value)
+            return;
+
+        this._indexes.insertAtIndex(value, index);
+    }
+
+    delete(value)
+    {
+        if (!this._validateIndex(value))
+            return false;
+
+        if (!this.size)
+            return false;
+
+        let index = this._indexes.lowerBound(value);
+        if (index === this._indexes.length)
+            return false;
+        this._indexes.splice(index, 1);
+        return true;
+    }
+
+    has(value)
+    {
+        if (!this.size)
+            return false;
+
+        let index = this._indexes.lowerBound(value);
+        return this._indexes[index] === value;
+    }
+
+    clear()
+    {
+        this._indexes = [];
+    }
+
+    indexGreaterThan(value)
+    {
+        const following = true;
+        return this._indexClosestTo(value, following);
+    }
+
+    indexLessThan(value)
+    {
+        const following = false;
+        return this._indexClosestTo(value, following);
+    }
+
+    [Symbol.iterator]()
+    {
+        return this._indexes[Symbol.iterator]();
+    }
+
+    // Private
+
+    _indexClosestTo(value, following)
+    {
+        if (!this.size)
+            return NaN;
+
+        if (this.size === 1) {
+            if (following)
+                return this.firstIndex > value ? this.firstIndex : NaN;
+            return this.firstIndex < value ? this.firstIndex : NaN;
+        }
+
+        let offset = following ? 1 : -1;
+        let position = this._indexes.lowerBound(value + offset);
+
+        let closestIndex = this._indexes[position];
+        if (closestIndex === undefined)
+            return NaN;
+
+        if (value === closestIndex)
+            return NaN;
+
+        if (!following && closestIndex > value)
+            return NaN;
+        return closestIndex;
+    }
+
+    _validateIndex(value)
+    {
+        console.assert(Number.isInteger(value) && value >= 0, "Index must be a non-negative integer.");
+        return Number.isInteger(value) && value >= 0;
+    }
+};
index 727a7ae..8d3936d 100644 (file)
 
     <script src="Base/WebInspector.js"></script>
     <script src="Base/Platform.js"></script>
+    <script src="Base/IndexSet.js"></script>
     <script src="Base/LinkedList.js"></script>
     <script src="Base/ListMultimap.js"></script>
     <script src="Base/Object.js"></script>
index 2d28c35..1f1f2e5 100644 (file)
@@ -37,6 +37,7 @@
 
     <script src="Base/WebInspector.js"></script>
     <script src="Base/Platform.js"></script>
+    <script src="Base/IndexSet.js"></script>
     <script src="Base/LinkedList.js"></script>
     <script src="Base/ListMultimap.js"></script>
     <script src="Base/Object.js"></script>
     <script src="Views/EditingSupport.js"></script>
     <script src="Views/View.js"></script>
 
+    <script src="Views/Resizer.js"></script>
+    <script src="Views/Table.js"></script>
+    <script src="Views/TableColumn.js"></script>
+
     <script type="text/javascript">
         WI.sharedApp = new WI.TestAppController;
         WI.sharedApp.initialize();
index aa12934..1acba8b 100644 (file)
@@ -101,6 +101,14 @@ WI.UIString = (string) => string;
 
 WI.indentString = () => "    ";
 
+WI.LayoutDirection = {
+    System: "system",
+    LTR: "ltr",
+    RTL: "rtl",
+};
+
+WI.resolvedLayoutDirection = () => { return InspectorFrontendHost.userInterfaceLayoutDirection(); }
+
 // Add stubs that are called by the frontend API.
 WI.updateDockedState = () => {};
 WI.updateDockingAvailability = () => {};
index c432dbe..e38ad72 100644 (file)
@@ -271,7 +271,7 @@ WI.NetworkTableContentView = class NetworkTableContentView extends WI.ContentVie
 
         if (this._table) {
             this._selectedResource = null;
-            this._table.clearSelectedRow();
+            this._table.deselectAll();
             this._table.reloadData();
             this._hidePopover();
             this._hideResourceDetailView();
@@ -285,7 +285,7 @@ WI.NetworkTableContentView = class NetworkTableContentView extends WI.ContentVie
         let rowIndex = this._rowIndexForResource(representedObject);
         if (rowIndex === -1) {
             this._selectedResource = null;
-            this._table.clearSelectedRow();
+            this._table.deselectAll();
             this._hideResourceDetailView();
             return;
         }
@@ -300,7 +300,7 @@ WI.NetworkTableContentView = class NetworkTableContentView extends WI.ContentVie
     networkResourceDetailViewClose(resourceDetailView)
     {
         this._selectedResource = null;
-        this._table.clearSelectedRow();
+        this._table.deselectAll();
         this._hideResourceDetailView();
     }
 
@@ -347,8 +347,9 @@ WI.NetworkTableContentView = class NetworkTableContentView extends WI.ContentVie
         return column === this._nameColumn;
     }
 
-    tableSelectedRowChanged(table, rowIndex)
+    tableSelectionDidChange(table)
     {
+        let rowIndex = table.selectedRow;
         if (isNaN(rowIndex)) {
             this._selectedResource = null;
             this._hideResourceDetailView();
@@ -1403,7 +1404,7 @@ WI.NetworkTableContentView = class NetworkTableContentView extends WI.ContentVie
         let rowIndex = this._rowIndexForResource(this._selectedResource);
         if (rowIndex === -1) {
             this._selectedResource = null;
-            this._table.clearSelectedRow();
+            this._table.deselectAll();
             return;
         }
 
index a6369e4..4741c9d 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-2018 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -126,11 +126,13 @@ body[dir=rtl] .table > .header > :matches(.sort-ascending, .sort-descending)::af
     line-height: 20px;
     vertical-align: middle;
     white-space: nowrap;
+    border-bottom: solid 1px transparent;
 }
 
 .table > .data-container > .data-list > li.selected {
     background-color: var(--selected-background-color-unfocused) !important;
     color: inherit !important;
+    background-clip: padding-box;
 }
 
 .table:focus > .data-container > .data-list li.selected {
@@ -176,11 +178,6 @@ body[dir=rtl] .table .cell:first-child {
 }
 
 @media (prefers-dark-interface) {
-    .table,
-    .table > .header {
-        background: var(--background-color);
-    }
-
     .table > .header > .sortable:active {
         background-color: hsla(0, 0%, var(--foreground-lightness), 0.2);
     }
index 839c06c..2a5488e 100644 (file)
@@ -87,6 +87,8 @@ WI.Table = class Table extends WI.View
         this._fillerHeight = 0; // Calculated in _resizeColumnsAndFiller.
 
         this._selectedRowIndex = NaN;
+        this._allowsMultipleSelection = false;
+        this._selectedRows = new WI.IndexSet;
 
         this._resizers = [];
         this._currentResizer = null;
@@ -122,6 +124,12 @@ WI.Table = class Table extends WI.View
     get delegate() { return this._delegate; }
     get rowHeight() { return this._rowHeight; }
     get selectedRow() { return this._selectedRowIndex; }
+
+    get selectedRows()
+    {
+        return Array.from(this._selectedRows);
+    }
+
     get scrollContainer() { return this._scrollContainerElement; }
 
     get sortOrder()
@@ -200,6 +208,24 @@ WI.Table = class Table extends WI.View
             this._dataSource.tableSortChanged(this);
     }
 
+    get allowsMultipleSelection()
+    {
+        return this._allowsMultipleSelection;
+    }
+
+    set allowsMultipleSelection(flag)
+    {
+        if (this._allowsMultipleSelection === flag)
+            return;
+
+        this._allowsMultipleSelection = flag;
+
+        let numberOfSelectedRows = this._selectedRows.size;
+        this._selectedRows.clear();
+        if (numberOfSelectedRows > 1)
+            this._notifySelectionDidChange();
+    }
+
     resize()
     {
         this._cachedWidth = NaN;
@@ -211,6 +237,7 @@ WI.Table = class Table extends WI.View
     reloadData()
     {
         this._cachedRows.clear();
+        this._selectedRows.clear();
 
         this._previousRevealedRowCount = NaN;
         this.needsLayout();
@@ -277,35 +304,71 @@ WI.Table = class Table extends WI.View
         this._cachedRows.delete(rowIndex);
     }
 
-    selectRow(rowIndex)
+    selectRow(rowIndex, extendSelection = false)
     {
-        if (this._selectedRowIndex === rowIndex)
+        console.assert(!extendSelection || this._allowsMultipleSelection, "Cannot extend selection with multiple selection disabled.");
+
+        if (this._isRowSelected(rowIndex)) {
+            if (!extendSelection)
+                this._deselectAllAndSelect(rowIndex);
             return;
+        }
 
-        let oldSelectedRow = this._cachedRows.get(this._selectedRowIndex);
-        if (oldSelectedRow)
-            oldSelectedRow.classList.remove("selected");
+        if (!extendSelection && this._selectedRows.size) {
+            this._suppressNextSelectionDidChange = true;
+            this.deselectAll();
+        }
 
         this._selectedRowIndex = rowIndex;
+        this._selectedRows.add(rowIndex);
 
         let newSelectedRow = this._cachedRows.get(this._selectedRowIndex);
         if (newSelectedRow)
             newSelectedRow.classList.add("selected");
 
-        if (this._delegate.tableSelectedRowChanged)
-            this._delegate.tableSelectedRowChanged(this, this._selectedRowIndex);
+        this._notifySelectionDidChange();
     }
 
-    clearSelectedRow()
+    deselectRow(rowIndex)
     {
-        if (isNaN(this._selectedRowIndex))
+        if (!this._isRowSelected(rowIndex))
             return;
 
-        let oldSelectedRow = this._cachedRows.get(this._selectedRowIndex);
-        if (oldSelectedRow)
-            oldSelectedRow.classList.remove("selected");
+        let oldSelectedRow = this._cachedRows.get(rowIndex);
+        if (!oldSelectedRow)
+            return;
 
-        this._selectedRowIndex = NaN;
+        oldSelectedRow.classList.remove("selected");
+
+        this._selectedRows.delete(rowIndex);
+
+        if (this._selectedRowIndex === rowIndex) {
+            this._selectedRowIndex = NaN;
+            if (this._selectedRows.size) {
+                // Find selected row closest to deselected row.
+                let preceding = this._selectedRows.indexLessThan(rowIndex);
+                let following = this._selectedRows.indexGreaterThan(rowIndex);
+
+                if (isNaN(preceding))
+                    this._selectedRowIndex = following;
+                else if (isNaN(following))
+                    this._selectedRowIndex = preceding;
+                else {
+                    if ((following - rowIndex) < (rowIndex - preceding))
+                        this._selectedRowIndex = following;
+                    else
+                        this._selectedRowIndex = preceding;
+                }
+            }
+        }
+
+        this._notifySelectionDidChange();
+    }
+
+    deselectAll()
+    {
+        const rowIndex = NaN;
+        this._deselectAllAndSelect(rowIndex);
     }
 
     columnWithIdentifier(identifier)
@@ -678,7 +741,7 @@ WI.Table = class Table extends WI.View
         let row = document.createElement("li");
         row.__index = rowIndex;
         row.__widthGeneration = 0;
-        if (rowIndex === this._selectedRowIndex)
+        if (this._isRowSelected(rowIndex))
             row.classList.add("selected");
 
         this._cachedRows.set(rowIndex, row);
@@ -1201,10 +1264,19 @@ WI.Table = class Table extends WI.View
         let column = this._visibleColumns[columnIndex];
         let rowIndex = row.__index;
 
+        if (this._isRowSelected(rowIndex)) {
+            if (event.metaKey)
+                this.deselectRow(rowIndex)
+            else
+                this._deselectAllAndSelect(rowIndex);
+            return;
+        }
+
         if (this._delegate.tableShouldSelectRow && !this._delegate.tableShouldSelectRow(this, cell, column, rowIndex))
             return;
 
-        this.selectRow(rowIndex);
+        let extendSelection = this._allowsMultipleSelection && event.metaKey;
+        this.selectRow(rowIndex, extendSelection);
     }
 
     _handleContextMenu(event)
@@ -1282,6 +1354,47 @@ WI.Table = class Table extends WI.View
             }, checked);
         }
     }
+
+    _deselectAllAndSelect(rowIndex)
+    {
+        if (!this._selectedRows.size)
+            return;
+
+        if (this._selectedRows.size === 1 && this._selectedRows.firstIndex === rowIndex)
+            return;
+
+        for (let selectedRowIndex of this._selectedRows) {
+            if (selectedRowIndex === rowIndex)
+                continue;
+            let oldSelectedRow = this._cachedRows.get(selectedRowIndex);
+            if (oldSelectedRow)
+                oldSelectedRow.classList.remove("selected");
+        }
+
+        this._selectedRowIndex = rowIndex;
+        this._selectedRows.clear();
+
+        if (!isNaN(rowIndex))
+            this._selectedRows.add(rowIndex);
+
+        this._notifySelectionDidChange();
+    }
+
+    _isRowSelected(rowIndex)
+    {
+        return this._selectedRows.has(rowIndex);
+    }
+
+    _notifySelectionDidChange()
+    {
+        if (this._suppressNextSelectionDidChange) {
+            this._suppressNextSelectionDidChange = false;
+            return;
+        }
+
+        if (this._delegate.tableSelectionDidChange)
+            this._delegate.tableSelectionDidChange(this);
+    }
 };
 
 WI.Table.SortOrder = {