Bug 176033

Summary:

Web Inspector: Styles Redesign: display "Inherited From" section headers

Product:

WebKit

Reporter:

Nikita Vasilyev <nvasilyev>

Component:

Web Inspector

Assignee:

Nikita Vasilyev <nvasilyev>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, hi, inspector-bugzilla-changes, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Bug Depends on:

175343

Bug Blocks:

Attachments:

Description	Flags
[Image] WIP	none
Patch	hi: review+
[Screenshot] With patch applied	none
Patch	none

Nikita Vasilyev

Reported 2017-08-28 13:11:05 PDT

In the current styles sidebar UI, we show them as: Media: all [rules inside of @media all {...}] Inherited From: .article [.article rule itself] Bug 175343 misses "Inherited From" and media query section headers.

Attachments
[Image] WIP (168.89 KB, image/png) 2017-09-02 20:48 PDT, Nikita Vasilyev	no flags	Details
Patch (5.17 KB, patch) 2017-09-05 16:52 PDT, Nikita Vasilyev	hi: review+	Details Formatted Diff Diff
[Screenshot] With patch applied (427.67 KB, image/png) 2017-09-05 16:57 PDT, Nikita Vasilyev	no flags	Details
Patch (4.81 KB, patch) 2017-09-05 19:47 PDT, Nikita Vasilyev	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2017-08-28 13:11:23 PDT

<rdar://problem/34116930>

Nikita Vasilyev

Comment 2 2017-09-02 19:35:36 PDT

To manually test this: 1. Open http://nv.github.io/webkit-inspector-bugs/styles-redesign/tests/inherited.html 2. Inspect h3#title Expected: In the styles sidebar, you should see sections in the following order: h3 {...} h3 🔒 {...} Inherited From: div.in .in {...} div {...} Inherited From: body.out .out {...}

Nikita Vasilyev

Comment 3 2017-09-02 20:48:50 PDT

Created attachment 319757 [details] [Image] WIP

Nikita Vasilyev

Comment 4 2017-09-05 16:52:25 PDT

Created attachment 319957 [details] Patch

Nikita Vasilyev

Comment 5 2017-09-05 16:57:21 PDT

Created attachment 319960 [details] [Screenshot] With patch applied

Devin Rousso

Comment 6 2017-09-05 18:15:11 PDT

Comment on attachment 319957 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=319957&action=review r=me > Source/WebInspectorUI/UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js:53 > + static createInheritedHeader(style) Is there a reason for this to be public static? > Source/WebInspectorUI/UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js:108 > + let inheritedHeader = WI.SpreadsheetRulesStyleDetailsPanel.createInheritedHeader(style); > + this.element.append(inheritedHeader); I don't think this needs to be in it's own variable, especially if you drop the static: this.element.appendChild(this._createInheritedHeader(style)); or even better, you could just do the append inside the function and name it something like `_appendInheritedHeader` :)

Nikita Vasilyev

Comment 7 2017-09-05 19:46:49 PDT

Comment on attachment 319957 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=319957&action=review >> Source/WebInspectorUI/UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js:53 >> + static createInheritedHeader(style) > > Is there a reason for this to be public static? No reason besides that it can be static since it's a pure function (has no side effects and return value is only determined by its input values). >> Source/WebInspectorUI/UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js:108 >> + this.element.append(inheritedHeader); > > I don't think this needs to be in it's own variable, especially if you drop the static: > > this.element.appendChild(this._createInheritedHeader(style)); > > or even better, you could just do the append inside the function and name it something like `_appendInheritedHeader` :) I like this.element.appendChild(this._createInheritedHeader(style)); Although _appendInheritedHeader would be a bit more brief, I find it more readable when all the append/addSubview operations are in this loop's body.

Nikita Vasilyev

Comment 8 2017-09-05 19:47:27 PDT

Created attachment 319970 [details] Patch

WebKit Commit Bot

Comment 9 2017-09-05 20:55:31 PDT

Comment on attachment 319970 [details] Patch Clearing flags on attachment: 319970 Committed r221662: <http://trac.webkit.org/changeset/221662>

WebKit Commit Bot

Comment 10 2017-09-05 20:55:33 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.