In "Styles — Rules", Option-clicking on a property name/value isn't a very common action. CSS rule's source links (on the right to CSS selectors) allow navigating to the corresponding line in CSS resource already. With CSS variables gaining popularity, I suggest repurposing Option-Click. In the following example, .title { color: var(--theme-text-color) } Option-clicking on "--theme-text-color" would jump to the variable's definition, e.g.: body { --theme-text-color: black } (via Bug 149521)
<rdar://problem/24593361>
Created attachment 271274 [details] [WIP] Patch So I was playing around with this for a bit, and I ran into something of a roadblock. It looks like CSSStyleDeclarations are only created for each instance of DOMNodeStyles, which means that CSS variables declared on a different tag won't have a style associated with them until the node is selected on which they are applied (look at UserInterface/Views/Variables.css for an example). I only spent a little time on this, so I may be missing an extremely obvious case for how to work around this, but what I have written so far works if the node which matches the rule containing the CSS variable is selected (thus creating the DOMNodeStyles) before trying to Option-click a CSS variable. If anyone else wants to take over, feel free as I am not sure how much time I have in the next few weeks (horray midterms >.> )
Comment on attachment 271274 [details] [WIP] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271274&action=review > Source/WebInspectorUI/ChangeLog:3 > + Web Inspector: In the styles sidebar, Option-clicking on --css-variable should jump to its definition This will require some thought. Technically, CSS variables, being properties, have an overriding cascade. For example: <style> :root { --my-color: blue; } div { --my-color: red; } .foo { color: var(--my-color); } </style> <div class="foo">Red</div> <p class="foo">Blue</p> However, I think in practice most variables will be global. This patch doesn't handle re-definitions properly. Perhaps that isn't required for a first take on the feature though. But ideally we would know and jump to precisely the appropriate --var declaration that applies.
I think we need a backend change to make this easy and correct. CSS variables should show up as a matched rule in the sidebar (at least inherited rules), but they don't. They do show up in the computed styles list, if you "show all". If they where in the matched rules cascade, this problem would be as simple as jump to property was for the computed styles goto arrow.
FWIW, Chrome just made them show up in the rules sidebar. https://twitter.com/ChromeDevTools/status/696742554402852865
That actually does not require a backend change. We control what counts as inherited on the front-end! I filed bug 154215.
With bug 154215 you should be able to jump to the right variable, just like computed style jumps.
(In reply to comment #7) > With bug 154215 you should be able to jump to the right variable, just like > computed style jumps. This was a very smart way of tackling this issue. Awesome catch! XD
Created attachment 271389 [details] Patch
Comment on attachment 271389 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271389&action=review > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js:1670 > + // Special case option clicking CSS variables. > + if (token && /\bvariable-2\b/.test(token.type)) { > + for (let rule of this._style.nodeStyles.matchedRules) { > + for (let property of rule.style.properties) { > + if (property.name === token.string && showSourceCode(rule.sourceCodeLocation, property.styleSheetTextRange)) > + return; > + } > + } > + for (let inherited of this._style.nodeStyles.inheritedRules) { > + for (let rule of inherited.matchedRules) { > + for (let property of rule.style.properties) { > + if (property.name === token.string && showSourceCode(rule.sourceCodeLocation, property.styleSheetTextRange)) > + return; > + } > + } > } > } You should not need to do this iteration. You should be able to use DOMNodeStyle's effectivePropertyForName. That will give you the property that is active.
Created attachment 271440 [details] Patch (In reply to comment #10) > Comment on attachment 271389 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=271389&action=review > > > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js:1670 > > + // Special case option clicking CSS variables. > > + if (token && /\bvariable-2\b/.test(token.type)) { > > + for (let rule of this._style.nodeStyles.matchedRules) { > > + for (let property of rule.style.properties) { > > + if (property.name === token.string && showSourceCode(rule.sourceCodeLocation, property.styleSheetTextRange)) > > + return; > > + } > > + } > > + for (let inherited of this._style.nodeStyles.inheritedRules) { > > + for (let rule of inherited.matchedRules) { > > + for (let property of rule.style.properties) { > > + if (property.name === token.string && showSourceCode(rule.sourceCodeLocation, property.styleSheetTextRange)) > > + return; > > + } > > + } > > } > > } > > You should not need to do this iteration. You should be able to use > DOMNodeStyle's effectivePropertyForName. That will give you the property > that is active. Totally forgot that that function existed. Thanks for the heads up!
Comment on attachment 271440 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271440&action=review > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js:1645 > + function showSourceCode(location, range) It is weird that you take a SourceCodeLocation but only use it for the sourceCode. You should change location to sourceCode.
Created attachment 271631 [details] Patch (In reply to comment #12) > Comment on attachment 271440 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=271440&action=review > > > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js:1645 > > + function showSourceCode(location, range) > > It is weird that you take a SourceCodeLocation but only use it for the > sourceCode. You should change location to sourceCode. I had only done that because I knew that both of the calls of showSourceCode (which I renamed to showRangeInSourceCode) had a ".sourceCode" member. This is also somewhat of a mixing of styleguides with one of my current classes, so it's my bad. Good catch though!
Comment on attachment 271631 [details] Patch Clearing flags on attachment: 271631 Committed r196746: <http://trac.webkit.org/changeset/196746>
All reviewed patches have been landed. Closing bug.