Bug 194608

Summary: Web Inspector: CSS Changes: only show changes for the given node
Product: WebKit Reporter: Devin Rousso <hi>
Component: Web InspectorAssignee: Devin Rousso <hi>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, hi, inspector-bugzilla-changes, mattbaker, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=201039
Bug Depends on:    
Bug Blocks: 195365    
Attachments:
Description Flags
Patch
none
[Image] Changes filter button - mockup
none
Patch none

Description Devin Rousso 2019-02-13 13:29:16 PST
There should be a way to toggle between showing all changes for the entire page and only showing changes for the selected node.
Comment 1 Radar WebKit Bug Importer 2019-02-13 13:29:36 PST
<rdar://problem/48050206>
Comment 2 Devin Rousso 2019-03-03 20:56:23 PST
Created attachment 363480 [details]
Patch
Comment 3 Matt Baker 2019-03-05 11:31:19 PST
Comment on attachment 363480 [details]
Patch

Being able to toggle this is great, but I think it needs to be more discoverable. What about a filter button? You'd have to place it in the header for the first file with changes, since there isn't a DetailsSection header.
Comment 4 Matt Baker 2019-03-05 11:31:51 PST
Created attachment 363661 [details]
[Image] Changes filter button - mockup
Comment 5 Matt Baker 2019-03-05 11:32:33 PST
(In reply to Matt Baker from comment #4)
> Created attachment 363661 [details]
> [Image] Changes filter button - mockup

It's not ideal placing the filter in the file header, since it appears that it applies just to that file. Maybe there is another approach we could take.
Comment 6 Timothy Hatcher 2019-03-06 10:47:12 PST
Comment on attachment 363480 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=363480&action=review

> Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js:220
> +        generalSettingsView.addSetting(WI.UIString("CSS:"), WI.settings.cssChangesPerNode, WI.UIString("Show only for selected node"));

The user labels for this is not clear that this is about source changes.
Comment 7 Devin Rousso 2019-03-06 10:53:29 PST
Comment on attachment 363480 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=363480&action=review

>> Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js:220
>> +        generalSettingsView.addSetting(WI.UIString("CSS:"), WI.settings.cssChangesPerNode, WI.UIString("Show only for selected node"));
> 
> The user labels for this is not clear that this is about source changes.

🤦‍♂️ good point.  I'll make the label "CSS Changes" instead.
Comment 8 Devin Rousso 2019-03-06 10:57:17 PST
Created attachment 363760 [details]
Patch
Comment 9 Devin Rousso 2019-03-06 11:01:02 PST
(In reply to Matt Baker from comment #3)
> Being able to toggle this is great, but I think it needs to be more discoverable. What about a filter button? You'd have to place it in the header for the first file with changes, since there isn't a DetailsSection header.

(In reply to Matt Baker from comment #5)
> It's not ideal placing the filter in the file header, since it appears that it applies just to that file. Maybe there is another approach we could take.

For now, I'd like to get the basic functionality landed so I can actually make use of it.  I've filed <https://webkit.org/b/195365> as a followup to create UI within the Changes panel for this.
Comment 10 Matt Baker 2019-03-06 11:06:00 PST
(In reply to Devin Rousso from comment #9)
> (In reply to Matt Baker from comment #3)
> > Being able to toggle this is great, but I think it needs to be more discoverable. What about a filter button? You'd have to place it in the header for the first file with changes, since there isn't a DetailsSection header.
> 
> (In reply to Matt Baker from comment #5)
> > It's not ideal placing the filter in the file header, since it appears that it applies just to that file. Maybe there is another approach we could take.
> 
> For now, I'd like to get the basic functionality landed so I can actually
> make use of it.  I've filed <https://webkit.org/b/195365> as a followup to
> create UI within the Changes panel for this.

Sounds good. If we add an Export button we'll need a toolbar, so I'm sure there will be an opportunity to add a button for this somewhere.
Comment 11 WebKit Commit Bot 2019-03-06 11:22:36 PST
Comment on attachment 363760 [details]
Patch

Clearing flags on attachment: 363760

Committed r242556: <https://trac.webkit.org/changeset/242556>
Comment 12 WebKit Commit Bot 2019-03-06 11:22:37 PST
All reviewed patches have been landed.  Closing bug.