996622 - [rule view] Use new theme colors for marking changed rules

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Brian Grinstead [:bgrins]]

I'm going to merge the three OS specific ruleview.css files into one, then update that one to use the new colors to show the green left border next to changed rules

Comment 1

[Brian Grinstead [:bgrins]]

Attachment: ruleview-changes.png

Not a huge change, just a 1px bigger border and using theme colors from https://developer.mozilla.org/en-US/docs/Tools/DevToolsColors to make the changes a bit more noticable

Comment 2

[Brian Grinstead [:bgrins]]

Attachment: ruleview-changes-pt1.patch

This patch just pulls the three separate ruleview.css files into one.  There were two differences that were only in OSX (Windows and Linux were the same):

In .ruleview-header there was a margin-top: 4px in Windows/Linux but not OSX.  I've wrapped this in an %ifndef XP_MACOSX to keep the same styling.
 
In OSX the following rule existed but wasn't in the others:

+.ruleview-rule-pseudo-element {
+  padding-left:20px;
+  border-left: solid 10px;
+}

This puts the border next to pseudo elements.  I'm not sure why it isn't in the others - I've added it back in unconditionally, but we could add it only in OSX if it has caused an issue.   I've pushed to try it out on windows/linux: https://tbpl.mozilla.org/?tree=Try&rev=5aabca06a2d2.

Comment 3

[Brian Grinstead [:bgrins]]

Attachment: ruleview-changes-pt2.patch

This uses the updated theme colors, and adds 1px extra to the border to call more attention to it.  I took the screenshot from attachment 8406876 [details] with both of these patches applied.

Comment 4

[Patrick Brosset <:pbro>]

Comment on attachment 8406884 [details] [diff] [review]
ruleview-changes-pt2.patch

Review of attachment 8406884 [details] [diff] [review]:
-----------------------------------------------------------------

The screenshot looks great.

Comment 5

[Brian Grinstead [:bgrins]]

https://hg.mozilla.org/integration/fx-team/rev/e1f4243f9636
https://hg.mozilla.org/integration/fx-team/rev/6f2321b72086

https://tbpl.mozilla.org/?tree=Fx-Team&rev=6f2321b72086

Comment 6

[Brian Grinstead [:bgrins]]

Comment on attachment 8406883 [details] [diff] [review]
ruleview-changes-pt1.patch

Review of attachment 8406883 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/themes/shared/devtools/ruleview.css
@@ +21,5 @@
> +  border-bottom-style: solid;
> +  padding: 1px 4px;
> +%ifndef XP_MACOSX
> +  margin-top: 4px;
> +%endif

For some reason having this condition end before the -moz-user-select causes test failures like: https://tbpl.mozilla.org/php/getParsedLog.php?id=37858954&tree=Fx-Team.  I will upload a new patch where this condition happens at the end of the rule

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Backed out for mochitest-dt failures on all platforms.
https://hg.mozilla.org/integration/fx-team/rev/814a00695b52

https://tbpl.mozilla.org/php/getParsedLog.php?id=37859950&tree=Fx-Team

Comment 8

[Brian Grinstead [:bgrins]]

Attachment: ruleview-changes-pt1.patch

Excited about how spectacularly I was able to break the test by changing only a couple lines of CSS.  Turns out I forgot to add the asterisk in front of the css to enable CSS preprocessing, so the following -moz-user-select property didn't get applied, which caused the test failures.  New push: https://tbpl.mozilla.org/?tree=Try&rev=fe3569221e41

Comment 9

[Brian Grinstead [:bgrins]]

https://hg.mozilla.org/integration/fx-team/rev/854848b9722d
https://hg.mozilla.org/integration/fx-team/rev/ba0fc8bc4aa0

https://tbpl.mozilla.org/?tree=Fx-Team&rev=ba0fc8bc4aa0

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/854848b9722d
https://hg.mozilla.org/mozilla-central/rev/ba0fc8bc4aa0