Title: [248480] trunk/Source/WebInspectorUI
Revision
248480
Author
[email protected]
Date
2019-08-09 14:59:29 -0700 (Fri, 09 Aug 2019)

Log Message

Web Inspector: Sources: increase the vertical space allocated to the call stack when paused
https://bugs.webkit.org/show_bug.cgi?id=200236

Reviewed by Joseph Pecoraro.

Rather than uniformly constrict the height of the Call Stack and Breakpoints sections, they
should "flex" based on their importance, which can likely be derived from the current state.
This way, it's possible to see information from each section at the same time, but still
have enough space in each section to be able to do something useful.

When paused, the most useful data is the call stack, so give the Call Stack section the most
vertical space (the Pause Reason is also important, but it usually needs very little space).

When not paused, it's likely that the user cares more about the resources with breakpoints
than those without, so favor the Breakpoints section.

Each section will only expand to fit it's maximum content height.

If the inspector window becomes too short, remove the "flex" entirely and have all the
content be part of a single scroll area instead.

* UserInterface/Views/SourcesNavigationSidebarPanel.js:
(WI.SourcesNavigationSidebarPanel):
(WI.SourcesNavigationSidebarPanel.prototype.createContentTreeOutline):
(WI.SourcesNavigationSidebarPanel.prototype._handleBreakpointElementAddedOrRemoved):
(WI.SourcesNavigationSidebarPanel.prototype._handleDebuggerPaused):
(WI.SourcesNavigationSidebarPanel.prototype._handleDebuggerResumed):
(WI.SourcesNavigationSidebarPanel.prototype._handleCallStackElementAddedOrRemoved): Deleted.
* UserInterface/Views/SourcesNavigationSidebarPanel.css:
(.sidebar > .panel.navigation.sources > .content > :matches(.pause-reason-container, .call-stack-container, .breakpoints-container)): Added.
(.sidebar > .panel.navigation.sources > .content .details-section): Added.
(.sidebar > .panel.navigation.sources > .content .details-section.collapsed > .header > .options, .sidebar > .panel.navigation.sources > .content .details-section:not(.collapsed) > .content, .sidebar > .panel.navigation.sources > .content .details-section:not(.collapsed) > .content > .group): Added.
(.sidebar > .panel.navigation.sources > .content > .breakpoints-container .create-breakpoint): Added.
(.sidebar > .panel.navigation.sources > .content > .navigation-bar): Added.
(@media (min-height: 650px)): Added.
(.sidebar > .panel.navigation.sources > .content > .pause-reason-container): Added.
(.sidebar > .panel.navigation.sources > .content > :matches(.call-stack-container, .breakpoints-container, .resources)): Added.
(.sidebar > .panel.navigation.sources > .content > .call-stack-container): Added.
(.sidebar > .panel.navigation.sources > .content > .breakpoints-container): Added.
(.sidebar > .panel.navigation.sources > .content > .resources): Added.
(.sidebar > .panel.navigation.sources > .content > .breakpoints-container .tree-outline .item.event-target-window .icon): Added.
(.sidebar > .panel.navigation.sources > .content > .details-section): Deleted.
(.sidebar > .panel.navigation.sources > .content > .details-section:matches(.paused-reason, .breakpoints).collapsed > .header > .options,): Deleted.
(.sidebar > .panel.navigation.sources > .content > .details-section.collapsed > .content): Deleted.
(.sidebar > .panel.navigation.sources > .content > .details-section.breakpoints > .header > .options .create-breakpoint): Deleted.
(.sidebar > .panel.navigation.sources > .content > .details-section:matches(.pause-reason, .call-stack, .breakpoints) > .content,): Deleted.
(.sidebar > .panel.navigation.sources > .content > .details-section:matches(.call-stack, .breakpoints) > .content): Deleted.
(.sidebar > .panel.navigation.sources > .content > .details-section:matches(.call-stack, .breakpoints):not(.collapsed) > .content): Deleted.
(.sidebar > .panel.navigation.sources > .content > .details-section.call-stack): Deleted.
(.sidebar > .panel.navigation.sources > .content > .details-section.breakpoints): Deleted.
(.sidebar > .panel.navigation.sources > .content > .details-section.breakpoints .create-breakpoint): Deleted.
(@media (min-height: 600px)): Deleted.
(.sidebar > .panel.navigation.sources > .content > .pause-reason): Deleted.
(.sidebar > .panel.navigation.sources > .content > .details-section:matches(.call-stack, .breakpoints):not(.collapsed) > .content,): Deleted.
(.sidebar > .panel.navigation.sources > .content > .details-section.breakpoints .tree-outline .item.event-target-window .icon): Deleted.
Wrap the Pause Reason, Call Stack, and Breakpoints `WI.DetailsSection`s in a container
element so that the styling of the sticky header doesn't get affected by the clamping of the
container's height.

* UserInterface/Views/DetailsSection.css:
(.details-section):
(.details-section > .header):
Create CSS variables for styles that will be overridden by the Sources navigation sidebar.

Modified Paths

Diff

Modified: trunk/Source/WebInspectorUI/ChangeLog (248479 => 248480)


--- trunk/Source/WebInspectorUI/ChangeLog	2019-08-09 21:57:26 UTC (rev 248479)
+++ trunk/Source/WebInspectorUI/ChangeLog	2019-08-09 21:59:29 UTC (rev 248480)
@@ -1,3 +1,69 @@
+2019-08-09  Devin Rousso  <[email protected]>
+
+        Web Inspector: Sources: increase the vertical space allocated to the call stack when paused
+        https://bugs.webkit.org/show_bug.cgi?id=200236
+
+        Reviewed by Joseph Pecoraro.
+
+        Rather than uniformly constrict the height of the Call Stack and Breakpoints sections, they
+        should "flex" based on their importance, which can likely be derived from the current state.
+        This way, it's possible to see information from each section at the same time, but still
+        have enough space in each section to be able to do something useful.
+
+        When paused, the most useful data is the call stack, so give the Call Stack section the most
+        vertical space (the Pause Reason is also important, but it usually needs very little space).
+
+        When not paused, it's likely that the user cares more about the resources with breakpoints
+        than those without, so favor the Breakpoints section.
+
+        Each section will only expand to fit it's maximum content height.
+
+        If the inspector window becomes too short, remove the "flex" entirely and have all the
+        content be part of a single scroll area instead.
+
+        * UserInterface/Views/SourcesNavigationSidebarPanel.js:
+        (WI.SourcesNavigationSidebarPanel):
+        (WI.SourcesNavigationSidebarPanel.prototype.createContentTreeOutline):
+        (WI.SourcesNavigationSidebarPanel.prototype._handleBreakpointElementAddedOrRemoved):
+        (WI.SourcesNavigationSidebarPanel.prototype._handleDebuggerPaused):
+        (WI.SourcesNavigationSidebarPanel.prototype._handleDebuggerResumed):
+        (WI.SourcesNavigationSidebarPanel.prototype._handleCallStackElementAddedOrRemoved): Deleted.
+        * UserInterface/Views/SourcesNavigationSidebarPanel.css:
+        (.sidebar > .panel.navigation.sources > .content > :matches(.pause-reason-container, .call-stack-container, .breakpoints-container)): Added.
+        (.sidebar > .panel.navigation.sources > .content .details-section): Added.
+        (.sidebar > .panel.navigation.sources > .content .details-section.collapsed > .header > .options, .sidebar > .panel.navigation.sources > .content .details-section:not(.collapsed) > .content, .sidebar > .panel.navigation.sources > .content .details-section:not(.collapsed) > .content > .group): Added.
+        (.sidebar > .panel.navigation.sources > .content > .breakpoints-container .create-breakpoint): Added.
+        (.sidebar > .panel.navigation.sources > .content > .navigation-bar): Added.
+        (@media (min-height: 650px)): Added.
+        (.sidebar > .panel.navigation.sources > .content > .pause-reason-container): Added.
+        (.sidebar > .panel.navigation.sources > .content > :matches(.call-stack-container, .breakpoints-container, .resources)): Added.
+        (.sidebar > .panel.navigation.sources > .content > .call-stack-container): Added.
+        (.sidebar > .panel.navigation.sources > .content > .breakpoints-container): Added.
+        (.sidebar > .panel.navigation.sources > .content > .resources): Added.
+        (.sidebar > .panel.navigation.sources > .content > .breakpoints-container .tree-outline .item.event-target-window .icon): Added.
+        (.sidebar > .panel.navigation.sources > .content > .details-section): Deleted.
+        (.sidebar > .panel.navigation.sources > .content > .details-section:matches(.paused-reason, .breakpoints).collapsed > .header > .options,): Deleted.
+        (.sidebar > .panel.navigation.sources > .content > .details-section.collapsed > .content): Deleted.
+        (.sidebar > .panel.navigation.sources > .content > .details-section.breakpoints > .header > .options .create-breakpoint): Deleted.
+        (.sidebar > .panel.navigation.sources > .content > .details-section:matches(.pause-reason, .call-stack, .breakpoints) > .content,): Deleted.
+        (.sidebar > .panel.navigation.sources > .content > .details-section:matches(.call-stack, .breakpoints) > .content): Deleted.
+        (.sidebar > .panel.navigation.sources > .content > .details-section:matches(.call-stack, .breakpoints):not(.collapsed) > .content): Deleted.
+        (.sidebar > .panel.navigation.sources > .content > .details-section.call-stack): Deleted.
+        (.sidebar > .panel.navigation.sources > .content > .details-section.breakpoints): Deleted.
+        (.sidebar > .panel.navigation.sources > .content > .details-section.breakpoints .create-breakpoint): Deleted.
+        (@media (min-height: 600px)): Deleted.
+        (.sidebar > .panel.navigation.sources > .content > .pause-reason): Deleted.
+        (.sidebar > .panel.navigation.sources > .content > .details-section:matches(.call-stack, .breakpoints):not(.collapsed) > .content,): Deleted.
+        (.sidebar > .panel.navigation.sources > .content > .details-section.breakpoints .tree-outline .item.event-target-window .icon): Deleted.
+        Wrap the Pause Reason, Call Stack, and Breakpoints `WI.DetailsSection`s in a container
+        element so that the styling of the sticky header doesn't get affected by the clamping of the
+        container's height.
+
+        * UserInterface/Views/DetailsSection.css:
+        (.details-section):
+        (.details-section > .header):
+        Create CSS variables for styles that will be overridden by the Sources navigation sidebar.
+
 2019-08-08  Devin Rousso  <[email protected]>
 
         Web Inspector: Page: don't allow the domain to be disabled

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/DetailsSection.css (248479 => 248480)


--- trunk/Source/WebInspectorUI/UserInterface/Views/DetailsSection.css	2019-08-09 21:57:26 UTC (rev 248479)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/DetailsSection.css	2019-08-09 21:59:29 UTC (rev 248480)
@@ -26,7 +26,10 @@
 .details-section {
     font-size: 10px;
     background-color: var(--panel-background-color);
-    border-bottom: 1px solid var(--border-color);
+    border-bottom: var(--details-section-border-bottom);
+
+    --details-section-border-bottom: 1px solid var(--border-color);
+    --details-section-header-top: -1px;
 }
 
 .details-section .details-section {
@@ -44,7 +47,7 @@
 
 .details-section > .header {
     position: sticky;
-    top: -1px;
+    top: var(--details-section-header-top);
     height: 23px;
     padding: 4px 0;
     -webkit-padding-end: 5px;

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.css (248479 => 248480)


--- trunk/Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.css	2019-08-09 21:57:26 UTC (rev 248479)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.css	2019-08-09 21:59:29 UTC (rev 248480)
@@ -65,56 +65,31 @@
     display: none;
 }
 
-.sidebar > .panel.navigation.sources > .content > .details-section {
-    padding-bottom: 1px;
+.sidebar > .panel.navigation.sources > .content > :matches(.pause-reason-container, .call-stack-container, .breakpoints-container) {
+    border-bottom: 1px solid var(--border-color);
+}
+
+.sidebar > .panel.navigation.sources > .content .details-section {
     font-size: 11px;
-    border-bottom-width: 1px !important;
+    --details-section-border-bottom: none;
 }
 
-.sidebar > .panel.navigation.sources > .content > .details-section:matches(.paused-reason, .breakpoints).collapsed > .header > .options,
-.sidebar > .panel.navigation.sources > .content > .details-section > .content,
-.sidebar > .panel.navigation.sources > .content > .details-section > .content > .group {
+.sidebar > .panel.navigation.sources > .content .details-section.collapsed > .header > .options,
+.sidebar > .panel.navigation.sources > .content .details-section:not(.collapsed) > .content,
+.sidebar > .panel.navigation.sources > .content .details-section:not(.collapsed) > .content > .group {
     display: block;
 }
 
-.sidebar > .panel.navigation.sources > .content > .details-section.collapsed > .content {
-    display: none;
-}
-
-.sidebar > .panel.navigation.sources > .content > .details-section.breakpoints > .header > .options .create-breakpoint {
+.sidebar > .panel.navigation.sources > .content > .breakpoints-container .create-breakpoint {
     width: 15px;
 }
 
-.sidebar > .panel.navigation.sources > .content > .details-section:matches(.pause-reason, .call-stack, .breakpoints) > .content,
-.sidebar > .panel.navigation.sources > .content > .tree-outline.resources {
-    overflow-y: auto;
-}
-
-.sidebar > .panel.navigation.sources > .content > .details-section:matches(.call-stack, .breakpoints) > .content {
-    /* Same calculation as below, but without var(--item-count) */
-    max-height: 110px;
-}
-
-.sidebar > .panel.navigation.sources > .content > .details-section:matches(.call-stack, .breakpoints):not(.collapsed) > .content {
-    /* 1px from .sidebar > .panel.navigation.sources > .content > .details-section (border-bottom) */
-    /* 5.5 to ensure that half of the next item is shown when overflowing */
-    min-height: calc(min(var(--item-count), 5.5) * 20px);
-}
-
-.sidebar > .panel.navigation.sources > .content > .details-section.call-stack {
-    --item-count: var(--call-stack-count);
-}
-
-.sidebar > .panel.navigation.sources > .content > .details-section.breakpoints {
-    --item-count: var(--breakpoints-count);
-}
-
-.sidebar > .panel.navigation.sources > .content > .details-section.breakpoints .create-breakpoint {
-    width: 15px;
-}
-
 .sidebar > .panel.navigation.sources > .content > .navigation-bar {
+    position: sticky;
+    top: 0;
+    z-index: var(--z-index-header);
     margin-bottom: 1px;
+    background-color: var(--panel-background-color);
 }
 
 .sidebar > .panel.navigation.sources > .content .tree-outline.single-thread {
@@ -125,7 +100,7 @@
     display: none;
 }
 
-@media (min-height: 600px) {
+@media (min-height: 650px) {
     .sidebar > .panel.navigation.sources > .content {
         display: flex;
         flex-direction: column;
@@ -132,17 +107,38 @@
         overflow-y: hidden;
     }
 
-    .sidebar > .panel.navigation.sources > .content > .pause-reason {
-        flex-shrink: 0;
+    .sidebar > .panel.navigation.sources > .content .details-section {
+        --details-section-header-top: 0;
     }
 
-    .sidebar > .panel.navigation.sources > .content > .details-section:matches(.call-stack, .breakpoints):not(.collapsed) > .content,
-    .sidebar > .panel.navigation.sources > .content > .tree-outline.resources {
+    .sidebar > .panel.navigation.sources > .content > .pause-reason-container {
+        flex-grow: 4;
+        flex-shrink: 1;
+    }
+
+    .sidebar > .panel.navigation.sources > .content > :matches(.call-stack-container, .breakpoints-container, .resources) {
         height: 100%;
+        max-height: fit-content;
+        overflow-y: auto;
     }
+
+    .sidebar > .panel.navigation.sources > .content > .call-stack-container {
+        flex-grow: 3;
+        flex-shrink: 2;
+    }
+
+    .sidebar > .panel.navigation.sources > .content > .breakpoints-container {
+        flex-grow: 2;
+        flex-shrink: 3;
+    }
+
+    .sidebar > .panel.navigation.sources > .content > .resources {
+        flex-grow: 1;
+        flex-shrink: 3;
+    }
 }
 
-.sidebar > .panel.navigation.sources > .content > .details-section.breakpoints .tree-outline .item.event-target-window .icon {
+.sidebar > .panel.navigation.sources > .content > .breakpoints-container .tree-outline .item.event-target-window .icon {
     content: url(../Images/TypeObject.svg);
 }
 
@@ -152,3 +148,4 @@
         background-color: var(--yellow-highlight-background-color);
     }
 }
+

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js (248479 => 248480)


--- trunk/Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js	2019-08-09 21:57:26 UTC (rev 248479)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js	2019-08-09 21:59:29 UTC (rev 248480)
@@ -113,9 +113,11 @@
         this._pauseReasonGroup = new WI.DetailsSectionGroup([this._pauseReasonTextRow]);
         this._pauseReasonSection = new WI.DetailsSection("paused-reason", WI.UIString("Pause Reason"), [this._pauseReasonGroup], this._pauseReasonLinkContainerElement);
 
+        this._pauseReasonContainer = document.createElement("div");
+        this._pauseReasonContainer.classList.add("pause-reason-container");
+        this._pauseReasonContainer.appendChild(this._pauseReasonSection.element);
+
         this._callStackTreeOutline = this.createContentTreeOutline({suppressFiltering: true});
-        this._callStackTreeOutline.addEventListener(WI.TreeOutline.Event.ElementAdded, this._handleCallStackElementAddedOrRemoved, this);
-        this._callStackTreeOutline.addEventListener(WI.TreeOutline.Event.ElementRemoved, this._handleCallStackElementAddedOrRemoved, this);
         this._callStackTreeOutline.addEventListener(WI.TreeOutline.Event.SelectionDidChange, this._handleTreeSelectionDidChange, this);
 
         let callStackRow = new WI.DetailsSectionRow;
@@ -124,6 +126,10 @@
         let callStackGroup = new WI.DetailsSectionGroup([callStackRow]);
         this._callStackSection = new WI.DetailsSection("call-stack", WI.UIString("Call Stack"), [callStackGroup]);
 
+        this._callStackContainer = document.createElement("div");
+        this._callStackContainer.classList.add("call-stack-container");
+        this._callStackContainer.appendChild(this._callStackSection.element);
+
         this._mainTargetTreeElement = null;
         this._activeCallFrameTreeElement = null;
 
@@ -193,11 +199,16 @@
 
         let breakpointsGroup = new WI.DetailsSectionGroup([breakpointsRow]);
         this._breakpointsSection = new WI.DetailsSection("breakpoints", WI.UIString("Breakpoints"), [breakpointsGroup], breakpointNavigationBarWrapper);
-        this.contentView.element.insertBefore(this._breakpointsSection.element, this.contentView.element.firstChild);
 
+        this._breakpointsContainer = document.createElement("div");
+        this._breakpointsContainer.classList.add("breakpoints-container");
+        this._breakpointsContainer.appendChild(this._breakpointsSection.element);
+
+        this.contentView.element.insertBefore(this._breakpointsContainer, this.contentView.element.firstChild);
+
         this._resourcesNavigationBar = new WI.NavigationBar;
         this.contentView.addSubview(this._resourcesNavigationBar);
-        this.contentView.element.insertBefore(this._resourcesNavigationBar.element, this._breakpointsSection.element.nextSibling);
+        this.contentView.element.insertBefore(this._resourcesNavigationBar.element, this._breakpointsContainer.nextSibling);
 
         this._resourcesNavigationBar.addNavigationItem(new WI.FlexibleSpaceNavigationItem);
 
@@ -488,7 +499,7 @@
 
     createContentTreeOutline(options = {})
     {
-        let treeOutline = super.createContentTreeOutline(options)
+        let treeOutline = super.createContentTreeOutline(options);
 
         treeOutline.addEventListener(WI.TreeOutline.Event.ElementRevealed, (event) => {
             let treeElement = event.data.element;
@@ -1532,19 +1543,6 @@
         console.error("Unknown tree element", treeElement);
     }
 
-    _handleCallStackElementAddedOrRemoved(event)
-    {
-        let count = this._callStackTreeOutline.children.length;
-        for (let child of this._callStackTreeOutline.children)
-            count += child.children.length;
-
-        // Don't count the main thread element when it is hidden.
-        if (WI.targets.length === 1)
-            --count;
-
-        this.element.style.setProperty("--call-stack-count", count);
-    }
-
     _handleBreakpointElementAddedOrRemoved(event)
     {
         let treeElement = event.data.element;
@@ -1578,11 +1576,6 @@
 
         if (setting)
             setting.value = !!treeElement.parent;
-
-        let count = this._breakpointsTreeOutline.children.length;
-        for (let child of this._breakpointsTreeOutline.children)
-            count += child.children.length;
-        this.element.style.setProperty("--breakpoints-count", count);
     }
 
     _populateCreateBreakpointContextMenu(contextMenu)
@@ -1794,10 +1787,10 @@
 
     _handleDebuggerPaused(event)
     {
-        this.contentView.element.insertBefore(this._callStackSection.element, this.contentView.element.firstChild);
+        this.contentView.element.insertBefore(this._callStackContainer, this.contentView.element.firstChild);
 
         if (this._updatePauseReason())
-            this.contentView.element.insertBefore(this._pauseReasonSection.element, this.contentView.element.firstChild);
+            this.contentView.element.insertBefore(this._pauseReasonContainer, this.contentView.element.firstChild);
 
         this._debuggerPauseResumeButtonItem.enabled = true;
         this._debuggerPauseResumeButtonItem.toggled = true;
@@ -1810,9 +1803,9 @@
 
     _handleDebuggerResumed(event)
     {
-        this._callStackSection.element.remove();
+        this._callStackContainer.remove();
 
-        this._pauseReasonSection.element.remove();
+        this._pauseReasonContainer.remove();
 
         this._debuggerPauseResumeButtonItem.enabled = true;
         this._debuggerPauseResumeButtonItem.toggled = false;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to