WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
176033
Web Inspector: Styles Redesign: display "Inherited From" section headers
https://bugs.webkit.org/show_bug.cgi?id=176033
Summary
Web Inspector: Styles Redesign: display "Inherited From" section headers
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.
Top of Page
Format For Printing
XML
Clone This Bug