⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 237495 in webkit


Ignore:
Timestamp:
Oct 27, 2018, 4:30:03 PM (8 years ago)
Author:
Matt Baker
Message:

Web Inspector: Table should support deleting rows
​https://bugs.webkit.org/show_bug.cgi?id=189803
<rdar://problem/44655709>

Reviewed by Devin Rousso.

Source/WebInspectorUI:

Add methods for removing rows from a Table without reloading the data
source. This patch adds Table.prototype.removeRow for removing a single
row, and Table.prototype.removeSelectedRows for removing the entire selection.
The latter also attempts to select a new row, if possible, before removing
the selection.

  • UserInterface/Base/IndexSet.js:

(WI.IndexSet.prototype.copy):

  • UserInterface/Views/Table.js:

(WI.Table):
(WI.Table.prototype.get numberOfRows):
Cache the number of rows in the table data source. Invalidate cached
value whenever table data is reloaded. Removing rows immediately updates
cached value, without incurring a potentially expensive reload.

(WI.Table.prototype.reloadData):
(WI.Table.prototype.selectRow):
(WI.Table.prototype.deselectRow):
(WI.Table.prototype.removeRow):
(WI.Table.prototype.removeSelectedRows):
Remove the selected rows and select a new row, if possible.
(WI.Table.prototype._updateVisibleRows):
(WI.Table.prototype._handleKeyDown):
(WI.Table.prototype._deselectAllAndSelect):
Drive-by fix: should work when rowToSelect isn't already selected.
(WI.Table.prototype._removeRows):
Remove rows and adjust the indexes of rows that are shifted up as a result
of preceding rows being removed.

LayoutTests:

  • inspector/table/resources/table-utilities.js:

(TestPage.registerInitializer.InspectorTest.TableDelegate.prototype.tableDidRemoveRows):
(TestPage.registerInitializer.createDataSource):
(TestPage.registerInitializer.InspectorTest.createTable):
(TestPage.registerInitializer.InspectorTest.createTableWithDelegate):
(TestPage.registerInitializer):
Update table test utilities to keep TableDataSource backing array in
sync when rows are removed. Also allow the number of table rows to be
specified when creating a table for testing.

  • inspector/table/table-remove-rows-expected.txt: Added.
  • inspector/table/table-remove-rows.html: Added.

Add tests for new Table methods removeRow(rowIndex) and removeSelectedRows().

  • inspector/unit-tests/index-set-expected.txt:
  • inspector/unit-tests/index-set.html:

Add tests for new IndexSet method copy().

Location:
trunk
Files:
2 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r237486 r237495  
     12018-10-27  Matt Baker  <mattbaker@apple.com>
     2
     3        Web Inspector: Table should support deleting rows
     4        https://bugs.webkit.org/show_bug.cgi?id=189803
     5        <rdar://problem/44655709>
     6
     7        Reviewed by Devin Rousso.
     8
     9        * inspector/table/resources/table-utilities.js:
     10        (TestPage.registerInitializer.InspectorTest.TableDelegate.prototype.tableDidRemoveRows):
     11        (TestPage.registerInitializer.createDataSource):
     12        (TestPage.registerInitializer.InspectorTest.createTable):
     13        (TestPage.registerInitializer.InspectorTest.createTableWithDelegate):
     14        (TestPage.registerInitializer):
     15        Update table test utilities to keep TableDataSource backing array in
     16        sync when rows are removed. Also allow the number of table rows to be
     17        specified when creating a table for testing.
     18
     19        * inspector/table/table-remove-rows-expected.txt: Added.
     20        * inspector/table/table-remove-rows.html: Added.
     21        Add tests for new Table methods `removeRow(rowIndex)` and `removeSelectedRows()`.
     22
     23        * inspector/unit-tests/index-set-expected.txt:
     24        * inspector/unit-tests/index-set.html:
     25        Add tests for new IndexSet method `copy()`.
     26
    1272018-10-26  Commit Queue  <commit-queue@webkit.org>
    228

  • trunk/LayoutTests/inspector/table/resources/table-utilities.js

    r237155 r237495  
    2222        }
    2323
     24        tableDidRemoveRows(table, rowIndexes)
     25        {
     26            // Prevent data source from getting out of sync.
     27            for (let index = rowIndexes.length - 1; index >= 0; --index)
     28                this.items.splice(index, 1);
     29        }
     30
    2431        tableSelectionDidChange(table)
    2532        {
    … …  
    3744    };
    3845
    39     function createDataSource() {
     46    function createDataSource(numberOfRows = 10) {
    4047        let items = [];
    41         for (let i = 0; i < 10; ++i)
     48        for (let i = 0; i < numberOfRows; ++i)
    4249            items.push({index: i, name: `Row ${i}`});
    4350
    … …  
    5966    }
    6067
    61     InspectorTest.createTable = function() {
    62         let dataSource = createDataSource();
     68    InspectorTest.createTable = function(numberOfRows) {
     69        let dataSource = createDataSource(numberOfRows);
    6370        let delegate = new InspectorTest.TableDelegate(dataSource.items);
    6471        return createTableInternal(dataSource, delegate);
    6572    };
    6673
    67     InspectorTest.createTableWithDelegate = function(delegate) {
     74    InspectorTest.createTableWithDelegate = function(delegate, numberOfRows) {
    6875        InspectorTest.assert(delegate instanceof InspectorTest.TableDelegate);
    6976
    70         let dataSource = createDataSource();
     77        let dataSource = createDataSource(numberOfRows);
    7178        delegate.items = dataSource.items;
    7279        return createTableInternal(dataSource, delegate);

  • trunk/LayoutTests/inspector/unit-tests/index-set-expected.txt

    r236853 r237495  
    6969PASS: Index less than 3 should be 2.
    7070
     71-- Running test case: IndexSet.prototype.copy
     72PASS: Copy and original should be different objects.
     73PASS: Copy and original should have the same values.
     74

  • trunk/LayoutTests/inspector/unit-tests/index-set.html

    r236853 r237495  
    195195    });
    196196
     197    suite.addTestCase({
     198        name: "IndexSet.prototype.copy",
     199        test() {
     200            let original = new WI.IndexSet([1, 2, 3]);
     201            let copied = original.copy();
     202            InspectorTest.expectNotEqual(copied, original, "Copy and original should be different objects.");
     203            InspectorTest.expectShallowEqual(Array.from(copied), Array.from(original), "Copy and original should have the same values.");
     204
     205            return true;
     206        }
     207    });
     208
    197209    suite.runTestCasesAndFinish();
    198210}

  • trunk/Source/WebInspectorUI/ChangeLog

    r237482 r237495  
     12018-10-27  Matt Baker  <mattbaker@apple.com>
     2
     3        Web Inspector: Table should support deleting rows
     4        https://bugs.webkit.org/show_bug.cgi?id=189803
     5        <rdar://problem/44655709>
     6
     7        Reviewed by Devin Rousso.
     8
     9        Add methods for removing rows from a Table without reloading the data
     10        source. This patch adds Table.prototype.removeRow for removing a single
     11        row, and Table.prototype.removeSelectedRows for removing the entire selection.
     12        The latter also attempts to select a new row, if possible, before removing
     13        the selection.
     14
     15        * UserInterface/Base/IndexSet.js:
     16        (WI.IndexSet.prototype.copy):
     17
     18        * UserInterface/Views/Table.js:
     19        (WI.Table):
     20        (WI.Table.prototype.get numberOfRows):
     21        Cache the number of rows in the table data source. Invalidate cached
     22        value whenever table data is reloaded. Removing rows immediately updates
     23        cached value, without incurring a potentially expensive reload.
     24
     25        (WI.Table.prototype.reloadData):
     26        (WI.Table.prototype.selectRow):
     27        (WI.Table.prototype.deselectRow):
     28        (WI.Table.prototype.removeRow):
     29        (WI.Table.prototype.removeSelectedRows):
     30        Remove the selected rows and select a new row, if possible.
     31        (WI.Table.prototype._updateVisibleRows):
     32        (WI.Table.prototype._handleKeyDown):
     33        (WI.Table.prototype._deselectAllAndSelect):
     34        Drive-by fix: should work when `rowToSelect` isn't already selected.
     35        (WI.Table.prototype._removeRows):
     36        Remove rows and adjust the indexes of rows that are shifted up as a result
     37        of preceding rows being removed.
     38
    1392018-10-26  Devin Rousso  <drousso@apple.com>
    240

  • trunk/Source/WebInspectorUI/UserInterface/Base/IndexSet.js

    r236853 r237495  
    9797    }
    9898
     99    copy()
     100    {
     101        let indexSet = new WI.IndexSet;
     102        indexSet._indexes = this._indexes.slice();
     103        return indexSet;
     104    }
     105
    99106    indexGreaterThan(value)
    100107    {

  • trunk/Source/WebInspectorUI/UserInterface/Views/Table.js

    r237016 r237495  
    7777
    7878        this._cachedRows = new Map;
     79        this._cachedNumberOfRows = NaN;
    7980
    8081        this._columnSpecs = new Map;
    … …  
    133134    get scrollContainer() { return this._scrollContainerElement; }
    134135
     136    get numberOfRows()
     137    {
     138        if (isNaN(this._cachedNumberOfRows))
     139            this._cachedNumberOfRows = this._dataSource.tableNumberOfRows(this);
     140
     141        return this._cachedNumberOfRows;
     142    }
     143
    135144    get sortOrder()
    136145    {
    … …  
    242251        this._selectedRows.clear();
    243252
     253        this._cachedNumberOfRows = NaN;
    244254        this._previousRevealedRowCount = NaN;
    245255        this.needsLayout();
    … …  
    310320    {
    311321        console.assert(!extendSelection || this._allowsMultipleSelection, "Cannot extend selection with multiple selection disabled.");
     322        console.assert(rowIndex >= 0 && rowIndex < this.numberOfRows);
    312323
    313324        if (this._isRowSelected(rowIndex)) {
    … …  
    334345    deselectRow(rowIndex)
    335346    {
     347        console.assert(rowIndex >= 0 && rowIndex < this.numberOfRows);
     348
    336349        if (!this._isRowSelected(rowIndex))
    337350            return;
    … …  
    374387    }
    375388
     389    removeRow(rowIndex)
     390    {
     391        console.assert(rowIndex >= 0 && rowIndex < this.numberOfRows);
     392
     393        if (this._isRowSelected(rowIndex))
     394            this.deselectRow(rowIndex);
     395
     396        this._removeRows(new WI.IndexSet([rowIndex]));
     397    }
     398
     399    removeSelectedRows()
     400    {
     401        let numberOfSelectedRows = this._selectedRows.size;
     402        if (!numberOfSelectedRows)
     403            return;
     404
     405        // Try selecting the row following the selection.
     406        let lastSelectedRow = this._selectedRows.lastIndex;
     407        let rowToSelect = lastSelectedRow + 1;
     408        if (rowToSelect === this.numberOfRows) {
     409            // If no row exists after the last selected row, try selecting a
     410            // deselected row (hole) within the selection.
     411            let firstSelectedRow = this._selectedRows.firstIndex;
     412            if (lastSelectedRow - firstSelectedRow > numberOfSelectedRows) {
     413                rowToSelect = this._selectedRows.firstIndex + 1;
     414                while (this._selectedRows.has(rowToSelect))
     415                    rowToSelect++;
     416            } else {
     417                // If the selection contains no holes, try selecting the row
     418                // preceding the selection.
     419                rowToSelect = firstSelectedRow > 0 ? firstSelectedRow - 1 : NaN;
     420            }
     421        }
     422
     423        // Change the selection before removing rows. This matches the behavior
     424        // of macOS Finder (in list and column modes) when removing selected items.
     425        let oldSelectedRows = this._selectedRows.copy();
     426        this._deselectAllAndSelect(rowToSelect);
     427        this._removeRows(oldSelectedRows);
     428    }
     429
    376430    columnWithIdentifier(identifier)
    377431    {
    … …  
    791845        let availableHeight = this._cachedHeight;
    792846
    793         let numberOfRows = this._dataSource.tableNumberOfRows(this);
     847        let numberOfRows = this.numberOfRows;
    794848        this._cachedNumberOfRows = numberOfRows;
    795849
    … …  
    10121066            return;
    10131067
    1014         let numberOfRows = this._dataSource.tableNumberOfRows(this);
     1068        let numberOfRows = this.numberOfRows;
    10151069        this._previousRevealedRowCount = numberOfRows;
    10161070
    … …  
    12231277            return;
    12241278
     1279        let numberOfRows = this.numberOfRows;
    12251280        let rowToSelect = NaN;
    12261281
    … …  
    13671422
    13681423        for (let selectedRowIndex of this._selectedRows) {
    1369             if (selectedRowIndex === rowIndex)
    1370                 continue;
    13711424            let oldSelectedRow = this._cachedRows.get(selectedRowIndex);
    13721425            if (oldSelectedRow)
    … …  
    13771430        this._selectedRows.clear();
    13781431
    1379         if (!isNaN(rowIndex))
     1432        if (!isNaN(rowIndex)) {
    13801433            this._selectedRows.add(rowIndex);
     1434            let newSelectedRow = this._cachedRows.get(rowIndex);
     1435            if (newSelectedRow)
     1436                newSelectedRow.classList.add("selected");
     1437        }
    13811438
    13821439        this._notifySelectionDidChange();
     1440    }
     1441
     1442    _removeRows(rowIndexes)
     1443    {
     1444        let removed = 0;
     1445
     1446        let adjustRowAtIndex = (index) => {
     1447            let newIndex = index - removed;
     1448            let row = this._cachedRows.get(index);
     1449            if (row) {
     1450                this._cachedRows.delete(row.__index);
     1451                row.__index = newIndex;
     1452                this._cachedRows.set(newIndex, row);
     1453            }
     1454
     1455            if (this._isRowSelected(index)) {
     1456                this._selectedRows.delete(index);
     1457                this._selectedRows.add(newIndex);
     1458                if (this._selectedRowIndex === index)
     1459                    this._selectedRowIndex = newIndex;
     1460            }
     1461        };
     1462
     1463        if (rowIndexes.has(this._selectedRowIndex))
     1464            this._selectedRowIndex = NaN;
     1465
     1466        for (let index = rowIndexes.firstIndex; index <= rowIndexes.lastIndex; ++index) {
     1467            if (rowIndexes.has(index)) {
     1468                let row = this._cachedRows.get(index);
     1469                if (row) {
     1470                    this._cachedRows.delete(index);
     1471                    row.remove();
     1472                }
     1473                removed++;
     1474                continue;
     1475            }
     1476
     1477            if (removed)
     1478                adjustRowAtIndex(index);
     1479        }
     1480
     1481        if (!removed)
     1482            return;
     1483
     1484        for (let index = rowIndexes.lastIndex + 1; index < this._cachedNumberOfRows; ++index)
     1485            adjustRowAtIndex(index);
     1486
     1487        this._cachedNumberOfRows -= removed;
     1488        console.assert(this._cachedNumberOfRows >= 0);
     1489
     1490        if (this._delegate.tableDidRemoveRows) {
     1491            this._delegate.tableDidRemoveRows(this, Array.from(rowIndexes));
     1492            console.assert(this._cachedNumberOfRows === this._dataSource.tableNumberOfRows(this), "Table data source should update after removing rows.");
     1493        }
    13831494    }
    13841495
Note: See TracChangeset for help on using the changeset viewer.