1012166 - Use svg files for itemArrow-* in DevTools sidemenu widget

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Tim Nguyen :ntim]

The light theme uses SVG, while the dark theme uses PNG images. It'd be nice to stay consistent. I'd say SVG is the best bet since it scales well at all levels.

Comment 1

[Tim Nguyen :ntim]

Attachment: itemArrow-dark-ltr.svg

Comment 2

[Tim Nguyen :ntim]

Attachment: itemArrow-dark-rtl.svg

Comment 3

[Tim Nguyen :ntim]

Attachment: Patch

Comment 4

[Tim Nguyen :ntim]

Attachment: Patch 1.1

For some reason the first patch didn't catch the removal of the png files.

Comment 5

[Brian Grinstead [:bgrins]]

Comment on attachment 8424372 [details] [diff] [review]
Patch 1.1

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

Looks good.  Will you push to try for this one before marking checkin-needed?

Comment 6

[Brian Grinstead [:bgrins]]

Went ahead and pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=bd2ab1bef338

Comment 7

[Tim Nguyen :ntim]

(In reply to Brian Grinstead [:bgrins] from comment #6)
> Went ahead and pushed to try:
> https://tbpl.mozilla.org/?tree=Try&rev=bd2ab1bef338

Thanks :) I don't have try yet. It's still awaiting server config.

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/89272013801d

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/89272013801d

Comment 10

[Tim Nguyen :ntim]

Comment on attachment 8424372 [details] [diff] [review]
Patch 1.1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): DevTools theming work
User impact if declined: Inconsistent file format, and bad scaling of side menu in devtools
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): Very low, image swap
String or IDL/UUID changes made by this patch: None

Comment 11

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8424372 [details] [diff] [review]
Patch 1.1

This seems like a nice to have change that can easily ride the trains rather than being uplifted. Unless these icons first landed in 31, I don't see a real value in taking the change at this point. Please do renom if you feel strongly that this change should be included in 31.