It would be useful, since I haven't memorized exactly which properties are shorthands and which are longhands. I was recently looking for "border-radius" in the sidebar but didn't find anything.
Created attachment 377195 [details] Patch
Created attachment 377196 [details] [Image] After Patch is applied
Created attachment 380784 [details] Patch
Created attachment 380785 [details] [Image] After Patch is applied
(In reply to Devin Rousso from comment #4) > Created attachment 380785 [details] > [Image] After Patch is applied Nice, I like having this as a toggle.
Comment on attachment 380784 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380784&action=review r=me, nice work! > Source/WebCore/inspector/InspectorStyleSheet.cpp:544 > + for (auto property : collectProperties(true)) { auto& will prevent copy constructing InspectorStyleProperty objects. > Source/WebCore/inspector/InspectorStyleSheet.cpp:581 > + IMO, separating the return value from the local with a blank line doesn't add anything. > Source/WebCore/inspector/InspectorStyleSheet.cpp:616 > + continue; if (m_style->getPropertyValue(name).isEmpty()) continue; > Source/WebInspectorUI/UserInterface/Models/CSSProperty.js:330 > + get isVariable() { return this._isVariable; } Nice. > Source/WebInspectorUI/UserInterface/Models/CSSProperty.js:391 > + if (this._isShorthand === undefined) { Our usual style is to reduce arrowing with an early return: if (this._isShorthand !== undefined) return this._isShorthand; > Source/WebInspectorUI/UserInterface/Views/ComputedStyleDetailsPanel.js:90 > + this._propertiesComputedStyleSection = new WI.ComputedStyleSection(this); I'd prefer to leave this as `_computedStyleSection`. I feel like it mostly adds churn. > Source/WebInspectorUI/UserInterface/Views/ComputedStyleSection.js:41 > + this._showsShorthandsInsteadOfLonghands = false; Since this is a concept, it should be singular. Reads better too: this._showsShorthandInsteadOfLonghand
Comment on attachment 380784 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380784&action=review >> Source/WebInspectorUI/UserInterface/Models/CSSProperty.js:391 >> + if (this._isShorthand === undefined) { > > Our usual style is to reduce arrowing with an early return: > > if (this._isShorthand !== undefined) > return this._isShorthand; I think this pattern is far more common for lazy-initialization. I also prefer this approach as it keeps the function "cleaner" in that there's only one `return`. >> Source/WebInspectorUI/UserInterface/Views/ComputedStyleSection.js:41 >> + this._showsShorthandsInsteadOfLonghands = false; > > Since this is a concept, it should be singular. Reads better too: > > this._showsShorthandInsteadOfLonghand If there was only one property per `WI.ComputedStyleSection`, I'd agree (show the shorthand instead of the longhand), but if there are multiple it makes more sense as a plural (show all shorthands instead of all longhands).
Created attachment 380809 [details] Patch Slimmed down this patch to avoid a bunch of unnecessary churn
Comment on attachment 380809 [details] Patch Clearing flags on attachment: 380809 Committed r251038: <https://trac.webkit.org/changeset/251038>
All reviewed patches have been landed. Closing bug.
<rdar://problem/56308811>