Clicking on the go-to arrow in Computed tab should switch to "Styles — Rules" panel and highlight the corresponding property, similarly to the old styles sidebar.
<rdar://problem/34986379>
Created attachment 323818 [details] Patch
Created attachment 323819 [details] [Animated GIF] With patch applied
Comment on attachment 323818 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=323818&action=review > Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.css:101 > +@keyframes style-property-highlight { > + from { background-color: yellow; } > +} Shouldn't this have some kind of `to`? > Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationSection.js:110 > + if (!this.didInitialLayout) > + this.updateLayout(); This seems peculiar. You should have a comment describing why we force a layout here. It seems you could do the same "defer until layout the property to highlight", but given this is a user action this probably isn't too bad. > Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:45 > + this._property.__view = this; __view is already used elsewhere. Every `Views/*View.js` sets __view. So I suggest using a different name to avoid potential conflicts. Maybe __spreadsheetStyleProperty or __propertyView.
Comment on attachment 323818 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=323818&action=review >> Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.css:101 >> +} > > Shouldn't this have some kind of `to`? I could add to { background-color: transparent; } but it works just fine without "to"! In the old styles sidebar we omitted "to" as well, see `@keyframes text-editor-highlight-fadeout`.
Created attachment 323854 [details] Patch Joe: you left cq-. Does it mean that I shouldn't cq+ the updated patch myself and wait for you to take another look?
(In reply to Nikita Vasilyev from comment #6) > Created attachment 323854 [details] > Patch > > Joe: you left cq-. Does it mean that I shouldn't cq+ the updated patch > myself and wait for you to take another look? cq- just means don't land the previous patch as is. If you have addressed the comments in the new patch you can cq+ or land the new patch given it has addressed the comments.
Comment on attachment 323854 [details] Patch Clearing flags on attachment: 323854 Committed r223333: <https://trac.webkit.org/changeset/223333>
All reviewed patches have been landed. Closing bug.