Attachment 357970 Details for Bug 192992 – Patch

[patch] Patch

bug-192992-20181221140143.patch (text/plain), 5.83 KB, created by Devin Rousso on 2018-12-21 13:01:44 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Devin Rousso

Created: 2018-12-21 13:01:44 PST

Size: 5.83 KB

patch

obsolete

>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index 0e46844587d5a3636397aad1c9d13c1dcc8c8db7..b9daf3dfe9e5737139e4a87823ae5963ef06dae9 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,23 @@
>+2018-12-21  Devin Rousso  <drousso@apple.com>
>+
>+        Web Inspector: update scroll position when revealing a virtualized WI.DataGridNode
>+        https://bugs.webkit.org/show_bug.cgi?id=192992
>+        <rdar://problem/46886427>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When `reveal`ing a `WI.DataGridNode`, if it is not currently in the DOM tree (e.g. it's been
>+        virtualized by it's owner `WI.DataGrid`), we need to scroll to it's position so that it gets
>+        added to the DOM tree before it can be revealed/selected.
>+
>+        * UserInterface/Views/DataGrid.js:
>+        (WI.DataGrid.prototype.layout):
>+        (WI.DataGrid.prototype.updateVisibleRows):
>+        (WI.DataGrid.prototype._updateVisibleRows): Deleted.
>+
>+        * UserInterface/Views/DataGridNode.js:
>+        (WI.DataGridNode.prototype.reveal):
>+
> 2018-12-20  Nikita Vasilyev  <nvasilyev@apple.com>
> 
>         Web Inspector: Dark Mode: Type profiler popovers have black text on dark background
>diff --git a/Source/WebInspectorUI/UserInterface/Views/DataGrid.js b/Source/WebInspectorUI/UserInterface/Views/DataGrid.js
>index 1355ed1f3a1bf7e454e93aabb333c58e674ff5d0..1c0ffb4bc88ca7a68ddddce6a5f411d917f99cd7 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/DataGrid.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/DataGrid.js
>@@ -876,7 +876,7 @@ WI.DataGrid = class DataGrid extends WI.View
>             this._updateHeaderAndScrollbar();
>         }
> 
>-        this._updateVisibleRows();
>+        this.updateVisibleRows();
>     }
> 
>     sizeDidChange()
>@@ -1047,7 +1047,7 @@ WI.DataGrid = class DataGrid extends WI.View
>         this.needsLayout();
>     }
> 
>-    _updateVisibleRows()
>+    updateVisibleRows(focusedDataGridNode)
>     {
>         if (this._inline || this._variableHeightRows) {
>             // Inline DataGrids always show all their rows, so we can't virtualize them.
>@@ -1066,6 +1066,9 @@ WI.DataGrid = class DataGrid extends WI.View
>                 nextElement = rowElement;
>             }
> 
>+            if (focusedDataGridNode)
>+                focusedDataGridNode.element.scrollIntoViewIfNeeded(false);
>+
>             return;
>         }
> 
>@@ -1079,26 +1082,32 @@ WI.DataGrid = class DataGrid extends WI.View
>         if (isNaN(this._cachedScrollableOffsetHeight))
>             this._cachedScrollableOffsetHeight = this._scrollContainerElement.offsetHeight;
> 
>-        let scrollTop = this._cachedScrollTop;
>-        let scrollableOffsetHeight = this._cachedScrollableOffsetHeight;
>+        let visibleRowCount = Math.ceil((this._cachedScrollableOffsetHeight + (overflowPadding * 2)) / rowHeight);
> 
>-        let visibleRowCount = Math.ceil((scrollableOffsetHeight + (overflowPadding * 2)) / rowHeight);
>+        if (!focusedDataGridNode) {
>+            let currentTopMargin = this._topDataTableMarginHeight;
>+            let currentBottomMargin = this._bottomDataTableMarginHeight;
>+            let currentTableBottom = currentTopMargin + (visibleRowCount * rowHeight);
> 
>-        let currentTopMargin = this._topDataTableMarginHeight;
>-        let currentBottomMargin = this._bottomDataTableMarginHeight;
>-        let currentTableBottom = currentTopMargin + (visibleRowCount * rowHeight);
>+            let belowTopThreshold = !currentTopMargin || this._cachedScrollTop > currentTopMargin + updateOffsetThreshold;
>+            let aboveBottomThreshold = !currentBottomMargin || this._cachedScrollTop + this._cachedScrollableOffsetHeight < currentTableBottom - updateOffsetThreshold;
> 
>-        let belowTopThreshold = !currentTopMargin || scrollTop > currentTopMargin + updateOffsetThreshold;
>-        let aboveBottomThreshold = !currentBottomMargin || scrollTop + scrollableOffsetHeight < currentTableBottom - updateOffsetThreshold;
>-
>-        if (belowTopThreshold && aboveBottomThreshold && !isNaN(this._previousRevealedRowCount))
>-            return;
>+            if (belowTopThreshold && aboveBottomThreshold && !isNaN(this._previousRevealedRowCount))
>+                return;
>+        }
> 
>         let revealedRows = this._rows.filter((row) => row.revealed && !row.hidden);
> 
>         this._previousRevealedRowCount = revealedRows.length;
> 
>-        let topHiddenRowCount = Math.max(0, Math.floor((scrollTop - overflowPadding) / rowHeight));
>+        if (focusedDataGridNode) {
>+            let focusedIndex = revealedRows.indexOf(focusedDataGridNode);
>+            let firstVisibleRowIndex = this._cachedScrollTop / rowHeight;
>+            if (focusedIndex < firstVisibleRowIndex || focusedIndex > firstVisibleRowIndex + visibleRowCount)
>+                this._scrollContainerElement.scrollTop = this._cachedScrollTop = (focusedIndex * rowHeight) - (this._cachedScrollableOffsetHeight / 2) + (rowHeight / 2);
>+        }
>+
>+        let topHiddenRowCount = Math.max(0, Math.floor((this._cachedScrollTop - overflowPadding) / rowHeight));
>         let bottomHiddenRowCount = Math.max(0, this._previousRevealedRowCount - topHiddenRowCount - visibleRowCount);
> 
>         let marginTop = topHiddenRowCount * rowHeight;
>diff --git a/Source/WebInspectorUI/UserInterface/Views/DataGridNode.js b/Source/WebInspectorUI/UserInterface/Views/DataGridNode.js
>index e99eed389d1a6b5c34c004c33e1eb9a3cdf04e14..e58ae5fdb80bd03a9ea93e58461761188b04098a 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/DataGridNode.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/DataGridNode.js
>@@ -544,7 +544,7 @@ WI.DataGridNode = class DataGridNode extends WI.Object
>             currentAncestor = currentAncestor.parent;
>         }
> 
>-        this.element.scrollIntoViewIfNeeded(false);
>+        this.dataGrid.updateVisibleRows(this);
> 
>         this.dispatchEventToListeners("revealed");
>     }

Actions: View | Formatted Diff | Diff

Attachments on bug 192992: 357970