Summary: | Web Inspector: Elements: Styles: move psuedo-selector rules before inherited rules | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Devin Rousso <hi> | ||||||||
Component: | Web Inspector | Assignee: | Devin Rousso <hi> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, hi, inspector-bugzilla-changes, joepeck, nvasilyev, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Devin Rousso
2019-07-19 10:36:41 PDT
I agree! I would prefer to have pseudo rules be right after the rules that match directly. Created attachment 374486 [details]
Patch
Created attachment 374487 [details]
Patch
Comment on attachment 374487 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=374487&action=review r=me! > Source/WebInspectorUI/UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js:306 > + beforePseudoId = InspectorBackend.domains.CSS.PseudoId.Before; You probably already know but this conflicts with your other change, so be careful when landing both. > Source/WebInspectorUI/UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js:331 > + if (style.inherited && (!previousStyle || !previousStyle.inherited)) > + addPseudoStyles(); Can this be simplified to just: if (style.inherited) addPsuedoStyles(); It seems like the `(!previousStyle || !previousStyle.inherited)` part is just to avoid calling the function again but the function already gracefully handles multiple times, so it will already do the work for us. Comment on attachment 374487 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=374487&action=review >> Source/WebInspectorUI/UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js:306 >> + beforePseudoId = InspectorBackend.domains.CSS.PseudoId.Before; > > You probably already know but this conflicts with your other change, so be careful when landing both. Yep :) >> Source/WebInspectorUI/UserInterface/Views/SpreadsheetRulesStyleDetailsPanel.js:331 >> + addPseudoStyles(); > > Can this be simplified to just: > > if (style.inherited) > addPsuedoStyles(); > > It seems like the `(!previousStyle || !previousStyle.inherited)` part is just to avoid calling the function again but the function already gracefully handles multiple times, so it will already do the work for us. Ooooo good point! Created attachment 375492 [details]
Patch
Comment on attachment 375492 [details] Patch Clearing flags on attachment: 375492 Committed r248204: <https://trac.webkit.org/changeset/248204> All reviewed patches have been landed. Closing bug. |