Bug 178329 - Web Inspector: [PARITY] Styles Redesign: Ability to add new style rules
Summary: Web Inspector: [PARITY] Styles Redesign: Ability to add new style rules
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-10-15 20:53 PDT by Nikita Vasilyev
Modified: 2017-12-06 00:24 PST (History)
5 users (show)

See Also:


Attachments
Patch (6.35 KB, patch)
2017-12-02 21:01 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (6.81 KB, patch)
2017-12-03 13:46 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (7.07 KB, patch)
2017-12-06 00:09 PST, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikita Vasilyev 2017-10-15 20:53:54 PDT
The old styles sidebar has a plus icon on the left to the filter field, which allows to add new style rules.

The new styles sidebar doesn't have it yet.
Comment 1 Radar WebKit Bug Importer 2017-10-15 20:54:07 PDT
<rdar://problem/35001005>
Comment 2 Devin Rousso 2017-12-02 12:29:22 PST
Any update on this?  I've been forced to use the old UI because I can't add new rules :(
Comment 3 Devin Rousso 2017-12-02 21:01:35 PST
Created attachment 328281 [details]
Patch
Comment 4 Nikita Vasilyev 2017-12-03 10:05:14 PST
Comment on attachment 328281 [details]
Patch

Thank you for workin on this, Devin!

It works as expected. Just a couple of nits and questions.

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

> Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:115
> +        if (this._focusSelectorElement)
> +            this.startEditingRuleSelector();

Maybe _shouldFocusSelectorElement or _pendingSelectorElementFocus.

> Source/WebInspectorUI/UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js:117
> +                section.startEditingRuleSelector();

What if several style declarations fall under this condition?

> Source/WebInspectorUI/UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js:183
> +        for (let styleSheet of styleSheets) {
> +            contextMenu.appendItem(styleSheet.displayName, () => {
> +                this._addNewRule(styleSheet.id);
> +            });
> +        }

I've noticed a peculiar behavior.

1. Open http://nv.github.io/webkit-inspector-bugs/styles-redesign/tests/color.html
2. Right click on the plus button. It shows "Available Style Sheets: color.css"
3. Click the plus button.
4. Right click on the plus button. It shows "Available Style Sheets: color.css, color.html"

Why is that? This happens in the old styles sidebar as well.

> Source/WebInspectorUI/UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js:213
> +    _addNewRule(stylesheetId) {

"{" should go on the next line.
Comment 5 Devin Rousso 2017-12-03 13:46:10 PST
Comment on attachment 328281 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js:117
>> +                section.startEditingRuleSelector();
> 
> What if several style declarations fall under this condition?

This is fine, because rules that are defined later in a stylesheet have a higher priority, so they'd appear closer to the top.

>> Source/WebInspectorUI/UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js:183
>> +        }
> 
> I've noticed a peculiar behavior.
> 
> 1. Open http://nv.github.io/webkit-inspector-bugs/styles-redesign/tests/color.html
> 2. Right click on the plus button. It shows "Available Style Sheets: color.css"
> 3. Click the plus button.
> 4. Right click on the plus button. It shows "Available Style Sheets: color.css, color.html"
> 
> Why is that? This happens in the old styles sidebar as well.

It appears that Inspector Style Sheets are created with the URL of the page.  "color.html" is the preferred Inspector Style Sheet.  I'll add some logic to separate it.
Comment 6 Devin Rousso 2017-12-03 13:46:51 PST
Created attachment 328301 [details]
Patch
Comment 7 WebKit Commit Bot 2017-12-05 23:55:29 PST
Comment on attachment 328301 [details]
Patch

Rejecting attachment 328301 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 328301, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
.webkit.org/git/WebKit
   5cf19e4..fd36e14  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 225568 = 5cf19e403b6b766c52fb8630564e59a3f608c9a4
r225569 = fd36e1404f49d6831c75faae3f9355dc7f913ac9
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: http://webkit-queues.webkit.org/results/5511591
Comment 8 Devin Rousso 2017-12-06 00:09:41 PST
Created attachment 328554 [details]
Patch
Comment 9 WebKit Commit Bot 2017-12-06 00:24:44 PST
Comment on attachment 328554 [details]
Patch

Clearing flags on attachment: 328554

Committed r225571: <https://trac.webkit.org/changeset/225571>
Comment 10 WebKit Commit Bot 2017-12-06 00:24:45 PST
All reviewed patches have been landed.  Closing bug.