Skip to content
Permalink
Browse files
Web Inspector: Computed Panel: Move ungrouped CSS variables to dedica…
…ted DetailsSidebar

https://bugs.webkit.org/show_bug.cgi?id=240805

Reviewed by Patrick Angle.

A follow-up from #782 (review)

> As an aside, it is unfortunate in hindsight that the variables section
> element can either directly contain variables for ungrouped mode, or contain
> sections. I wonder if there is a better way to handle this where we never
> have to even add the variables section to the map and instead have a single
> "section" within it (without a header) that behaves just like the grouped
> sections, but for ungrouped mode.

The top-level Variables details section contains either:
- the list of ungrouped variables
- nested detail sections for each group of variables grouped by type

When toggling between grouping modes, we remove the host details sections for
variable groups, but we need to take care not to remove the top-level Variables
details section. This isn't ideal. All variable lists, ungrouped or grouped,
need their own independent host details section that can be safely removed
without impacting others.

A details section has a header with text and a button to toggle collapsing the
section contents. Ungrouped variables don't need this. The top-level Variables
details section serves this role (the reason why the code was written with it
doing double duty).

We can change the implementation of `WI.DetailsSection` to hide the header
if a title is not provided. This enables us to nest details sections without
showing extraneous headers. This relieves `WI.ComputedStyleDetailsPanel.layout()`
of the duty to avoid removing the top-level Variables details section.

* Source/WebInspectorUI/UserInterface/Views/ComputedStyleDetailsPanel.css:
(.sidebar > .panel.details.css-style > .content > .computed > .details-section.computed-style-variables .scope-bar.computed-style-variables-grouping-mode:hover):
(.sidebar > .panel.details.css-style > .content > .computed > .details-section.computed-style-variables .computed-style-section):

Adding symmetric padding to the top of the section to space out the contents
from the header's bottom border.

* Source/WebInspectorUI/UserInterface/Views/ComputedStyleDetailsPanel.js:
(WI.ComputedStyleDetailsPanel.prototype.layout):
(WI.ComputedStyleDetailsPanel.prototype._renderVariablesStyleSectionGroup):
* Source/WebInspectorUI/UserInterface/Views/DetailsSection.css:
(.details-section > .header:not(:empty)):
(.details-section .details-section > .header:not(:empty)):
(.details-section .details-section:not(.collapsed) > .header:not(:empty)):
(.details-section > .header:not(:empty)::before):
(.details-section > .header): Deleted.
(.details-section .details-section > .header): Deleted.
(.details-section .details-section:not(.collapsed) > .header): Deleted.
(.details-section > .header::before): Deleted.
* Source/WebInspectorUI/UserInterface/Views/DetailsSection.js:
(WI.DetailsSection):
(WI.DetailsSection.prototype.get title):
(WI.DetailsSection.prototype.set title):

Canonical link: https://commits.webkit.org/250956@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294800 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
rcaliman-apple committed May 25, 2022
1 parent f6f23c0 commit 6325b47cb27c23328f3eeadb13ae676777bb4974
Showing 4 changed files with 16 additions and 19 deletions.
@@ -86,4 +86,8 @@

.sidebar > .panel.details.css-style > .content > .computed > .details-section.computed-style-variables .scope-bar.computed-style-variables-grouping-mode:hover {
--scope-bar-background-opacity-override: 0.5;
}
}

.sidebar > .panel.details.css-style > .content > .computed > .details-section.computed-style-variables .computed-style-section {
padding-top: var(--computed-style-section-vertical-padding);
}
@@ -216,10 +216,7 @@ WI.ComputedStyleDetailsPanel = class ComputedStyleDetailsPanel extends WI.StyleD
this.removeSubview(styleSection);
styleSection.element.remove();
this._detailsSectionByStyleSectionMap.delete(styleSection);

// The top-level details section for variables needs to be preserved because it's the host of nested details sections for variables groups.
if (detailsSection !== this._variablesSection)
detailsSection.element.remove();
detailsSection.element.remove();
}

this._variablesStyleSectionForGroupTypeMap.clear();
@@ -257,19 +254,12 @@ WI.ComputedStyleDetailsPanel = class ComputedStyleDetailsPanel extends WI.StyleD

this.addSubview(variablesStyleSection);

let detailsSection;
if (!label) {
this._variablesRow.element.appendChild(variablesStyleSection.element);
detailsSection = this._variablesSection;
} else {
let detailsSectionRow = new WI.DetailsSectionRow;
let detailsSectionGroup = new WI.DetailsSectionGroup([detailsSectionRow]);
detailsSection = new WI.DetailsSection(`computed-style-variables-group-${groupType}`, label, [detailsSectionGroup]);
detailsSection.addEventListener(WI.DetailsSection.Event.CollapsedStateChanged, this._handleDetailsSectionCollapsedStateChanged, this);

detailsSectionRow.element.appendChild(variablesStyleSection.element);
this._variablesRow.element.appendChild(detailsSection.element);
}
let detailsSectionRow = new WI.DetailsSectionRow;
let detailsSectionGroup = new WI.DetailsSectionGroup([detailsSectionRow]);
let detailsSection = new WI.DetailsSection(`computed-style-variables-group-${groupType}`, label, [detailsSectionGroup]);
detailsSection.addEventListener(WI.DetailsSection.Event.CollapsedStateChanged, this._handleDetailsSectionCollapsedStateChanged, this);
detailsSectionRow.element.appendChild(variablesStyleSection.element);
this._variablesRow.element.appendChild(detailsSection.element);

this._detailsSectionByStyleSectionMap.set(variablesStyleSection, detailsSection);
this._variablesStyleSectionForGroupTypeMap.set(groupType, variablesStyleSection);
@@ -24,9 +24,11 @@
*/

.computed-style-section {
padding-bottom: 3px;
padding-bottom: var(--computed-style-section-vertical-padding);
color: var(--text-color-tertiary);
user-select: text;

--computed-style-section-vertical-padding: 3px;
}

.computed-style-section .computed-property-item {
@@ -78,6 +78,7 @@ WI.DetailsSection = class DetailsSection extends WI.Object

set title(title)
{
this._headerElement.toggleAttribute("hidden", !title);
this._titleElement.textContent = title;
}

0 comments on commit 6325b47

Please sign in to comment.