Bug 199950 – Web Inspector: Elements: Styles: move psuedo-selector rules before inherited rules

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Devin Rousso]

Since pseudo-selector rules (usually) affect the selected element, it's more useful to have them near that element's rules than after all of it's inherited rules.

[Nikita Vasilyev]

I agree! I would prefer to have pseudo rules be right after the rules that match directly.

[Devin Rousso]

Created attachment 374486 [details]
Patch

[Devin Rousso]

Created attachment 374487 [details]
Patch

[Joseph Pecoraro]

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.

[Devin Rousso]

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!

[Devin Rousso]

Created attachment 375492 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 375492 [details]
Patch

Clearing flags on attachment: 375492

Committed r248204: <https://trac.webkit.org/changeset/248204>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/53907105>