Bug 174329

Summary: Web Inspector: [PARITY] Styles: Command-click on a property name should jump to definition in Resources tab
Product: WebKit Reporter: Nikita Vasilyev <nvasilyev>
Component: Web InspectorAssignee: Nikita Vasilyev <nvasilyev>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, inspector-bugzilla-changes, joepeck, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on: 145982    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
[Animated GIF] With patch applied none

Nikita Vasilyev
Reported 2017-07-10 16:33:00 PDT
In Rules panel, clicking on a CSS property while holding Command key, should navigate to corresponding line number in Resources tab. Currently, we use Option-click for that. Command-click inserts a second text caret (implemented by CodeMirror).
Attachments
Patch (5.18 KB, patch)
2017-10-13 22:34 PDT, Nikita Vasilyev 		no flags
DetailsFormatted DiffDiff
[Animated GIF] With patch applied (124.52 KB, image/gif)
2017-10-13 22:42 PDT, Nikita Vasilyev 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2017-07-10 16:33:26 PDT
<rdar://problem/33225564>
Nikita Vasilyev
Comment 2 2017-10-13 22:34:44 PDT
Created attachment 323794 [details] Patch
Nikita Vasilyev
Comment 3 2017-10-13 22:42:36 PDT
Created attachment 323795 [details] [Animated GIF] With patch applied
Joseph Pecoraro
Comment 4 2017-10-15 15:15:28 PDT
Comment on attachment 323794 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=323794&action=review > Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:265 > + let range = this._property.styleSheetTextRange; > + const options = { > + ignoreNetworkTab: true, > + ignoreSearchTab: true, > + }; > + let sourceCode = sourceCodeLocation.sourceCode; > + WI.showSourceCodeLocation(sourceCode.createSourceCodeLocation(range.startLine, range.startColumn), options); This looks like it would always take you to the styleSheetTextRange's start. But what would be more useful would be the key/value you clicked on's location. This is most useful when using Sass/Less with SourceMaps. Individual properties within a rule might come from a Mixin, and using the location of the property itself will get your to that Mixin's definition but other normal properties would take you to the rule's definition.
Nikita Vasilyev
Comment 5 2017-10-15 15:29:47 PDT
View in context: https://bugs.webkit.org/attachment.cgi?id=323794&action=review >> Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:265 >> + WI.showSourceCodeLocation(sourceCode.createSourceCodeLocation(range.startLine, range.startColumn), options); > > This looks like it would always take you to the styleSheetTextRange's start. But what would be more useful would be the key/value you clicked on's location. This is most useful when using Sass/Less with SourceMaps. Individual properties within a rule might come from a Mixin, and using the location of the property itself will get your to that Mixin's definition but other normal properties would take you to the rule's definition. This takes you to the property start (e.g. property name). This is what the current styles sidebar does, I pretty much copy/pasted the code from WI.CSSStyleDeclarationTextEditor.prototype.tokenTrackingControllerHighlightedRangeWasClicked. I agree this can be improved. Currently, there's CSSProperty.prototype._styleSheetTextRange, but no range for property value, AFAIK. I'd prefer to work on it later in a follow up issue.
Joseph Pecoraro
Comment 6 2017-10-16 11:09:56 PDT
(In reply to Nikita Vasilyev from comment #5) > This takes you to the property start (e.g. property name). Ahh! I misread, I thought it was the rule's start. This should be good enough to get expected behavior.
Joseph Pecoraro
Comment 7 2017-10-16 11:11:33 PDT
Comment on attachment 323794 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=323794&action=review r=me > Source/WebInspectorUI/UserInterface/Base/Main.js:1460 > + if (metaKeyDidChange) > + document.body.classList.toggle("meta-key-pressed", this.modifierKeys.metaKey); I wonder how this will affect style recalculations changing a global property like this. Lets keep an eye out for any possible stutters after this lands.
WebKit Commit Bot
Comment 8 2017-10-17 11:39:51 PDT
Comment on attachment 323794 [details] Patch Clearing flags on attachment: 323794 Committed r223561: <https://trac.webkit.org/changeset/223561>
WebKit Commit Bot
Comment 9 2017-10-17 11:39:52 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.