modify add-on entry design in Add-ons Manager to match InContent prefs styling

RESOLVED FIXED in Firefox 45

Status

()

defect
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: designakt, Assigned: ntim)

Tracking

(Blocks 1 bug)

unspecified
mozilla45
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox45 fixed)

Details

Attachments

(5 attachments, 4 obsolete attachments)

With a view small modification we can make the add-on entries align better with the new InContent prefs styling.
- replacing the striped background with a gradient:
	background-image: radial-gradient(50% 100%, <color> 0%, #FBFBFB 100%);
	<color> depending on state:
		#FFE8E9	error 
		#F9F5E5	warning
		#EFFAF2	pending-install
		#F2F2F2	pending-uninstall
- Notification Symbols Flat
- Generic Extension Icon as used in the new add-on install flow
svg of the 4 notification symbols
Reporter

Updated

4 years ago
Blocks: 1175290
Assignee

Comment 2

4 years ago
Posted patch Patch (obsolete) — Splinter Review
Assignee: nobody → ntim.bugs
Status: NEW → ASSIGNED
Attachment #8675391 - Flags: review?(dao)
Assignee

Comment 3

4 years ago
Posted patch Patch v1.1 (obsolete) — Splinter Review
Changed extensionGeneric.svg to make sure it still has the same dimensions as before.
Attachment #8675391 - Attachment is obsolete: true
Attachment #8675391 - Flags: review?(dao)
Assignee

Updated

4 years ago
Attachment #8675393 - Flags: review?(dao)
Comment on attachment 8675393 [details] [diff] [review]
Patch v1.1

>--- a/toolkit/themes/linux/mozapps/jar.mn
>+++ b/toolkit/themes/linux/mozapps/jar.mn
>@@ -8,35 +8,40 @@ toolkit.jar:
> + skin/classic/mozapps/downloads/downloads.css             (downloads/downloads.css)
> * skin/classic/mozapps/extensions/extensions.css           (extensions/extensions.css)
> + skin/classic/mozapps/extensions/category-search.png      (extensions/category-search.png)
> + skin/classic/mozapps/extensions/category-discover.png    (extensions/category-discover.png)
> + skin/classic/mozapps/extensions/category-plugins.png     (extensions/category-plugins.png)
> + skin/classic/mozapps/extensions/category-service.png     (extensions/category-service.png)
> + skin/classic/mozapps/extensions/category-recent.png      (extensions/category-recent.png)
> + skin/classic/mozapps/extensions/category-available.png   (extensions/category-available.png)
>-+ skin/classic/mozapps/extensions/extensionGeneric.png     (extensions/extensionGeneric.png)
>++ skin/classic/mozapps/extensions/extensionGeneric.svg     (../../shared/extensions/extensionGeneric.svg)
> + skin/classic/mozapps/extensions/extensionGeneric-16.png  (extensions/extensionGeneric-16.png)
> + skin/classic/mozapps/extensions/dictionaryGeneric.png    (extensions/dictionaryGeneric.png)
> + skin/classic/mozapps/extensions/dictionaryGeneric-16.png (extensions/dictionaryGeneric-16.png)
> + skin/classic/mozapps/extensions/themeGeneric.png         (extensions/themeGeneric.png)
> + skin/classic/mozapps/extensions/themeGeneric-16.png      (extensions/themeGeneric-16.png)
> + skin/classic/mozapps/extensions/localeGeneric.png        (extensions/localeGeneric.png)
> * skin/classic/mozapps/extensions/newaddon.css             (extensions/newaddon.css)
> + skin/classic/mozapps/extensions/selectAddons.css         (extensions/selectAddons.css)
> + skin/classic/mozapps/extensions/heart.png                (extensions/heart.png)
> + skin/classic/mozapps/extensions/navigation.png           (../../shared/extensions/navigation.png)
>++ skin/classic/mozapps/extensions/alerticon-warning.svg    (../../shared/extensions/alerticon-warning.svg)
>++ skin/classic/mozapps/extensions/alerticon-error.svg      (../../shared/extensions/alerticon-error.svg)
>++ skin/classic/mozapps/extensions/alerticon-info-positive.svg (../../shared/extensions/alerticon-info-positive.svg)
>++ skin/classic/mozapps/extensions/alerticon-info-negative.svg (../../shared/extensions/alerticon-info-negative.svg)

please move this to toolkit/themes/shared/jar.inc.mn

>--- a/toolkit/themes/shared/extensions/newaddon.inc.css
>+++ b/toolkit/themes/shared/extensions/newaddon.inc.css
>@@ -35,17 +35,17 @@
>   margin: 25px 10px;
> }
> 
> #icon {
>   margin-top: 8px;
>   -moz-margin-end: 10px;
>   max-width: 64px;
>   max-height: 64px;
>-  list-style-image: url("chrome://mozapps/skin/extensions/extensionGeneric.png");
>+  list-style-image: url("chrome://mozapps/skin/extensions/extension-generic.svg");

typo
Attachment #8675393 - Flags: review?(dao) → review-
Assignee

Comment 5

4 years ago
The mozapps jar.mn doesn't have its shared file yet, should I create one or should I move it to another bug ?
Assignee

Comment 6

4 years ago
(In reply to Tim Nguyen [:ntim] from comment #5)
> The mozapps jar.mn doesn't have its shared file yet, should I create one or
> should I move it to another bug ?

"it": the task.
Flags: needinfo?(dao)
mozapps *sigh*

Please create the shared manifest.
Flags: needinfo?(dao)
Assignee

Comment 8

4 years ago
Posted patch Patch v1.2 (obsolete) — Splinter Review
Fixed the typo
Attachment #8675393 - Attachment is obsolete: true
Attachment #8677058 - Flags: review?(dao)
Comment on attachment 8677058 [details] [diff] [review]
Patch v1.2

Do we still need extensionGeneric-16.png or can we use the SVG instead?
Attachment #8677058 - Flags: review?(dao) → review+
Attachment #8677059 - Flags: review?(dao) → review+
Assignee

Comment 11

4 years ago
(In reply to Dão Gottwald [:dao] from comment #10)
> Comment on attachment 8677058 [details] [diff] [review]
> Patch v1.2
> 
> Do we still need extensionGeneric-16.png or can we use the SVG instead?
It's used for a different purpose (urlbar icon on linux and page favicon for various add-on related pages). So I would rather replace it in another bug.
Assignee

Updated

4 years ago
Keywords: checkin-needed
Assignee

Comment 12

4 years ago
(In reply to Tim Nguyen [:ntim] from comment #11)
> (In reply to Dão Gottwald [:dao] from comment #10)
> > Comment on attachment 8677058 [details] [diff] [review]
> > Patch v1.2
> > 
> > Do we still need extensionGeneric-16.png or can we use the SVG instead?
> It's used for a different purpose (urlbar icon on linux and page favicon for
> various add-on related pages). So I would rather replace it in another bug.

Filed bug 1217347
Blocks: 1217347
sorry had to back this out for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=5264485&repo=fx-team
Flags: needinfo?(ntim.bugs)
Assignee

Comment 16

4 years ago
Posted patch Patch v2 (Part 1) (obsolete) — Splinter Review
Attachment #8677058 - Attachment is obsolete: true
Flags: needinfo?(ntim.bugs)
Assignee

Comment 17

4 years ago
(In reply to Tim Nguyen [:ntim] from comment #16)
> Created attachment 8677483 [details] [diff] [review]
> Patch v2

Looks like I forgot to remove extensionGeneric-XP.png from the Windows manifest. This new patch should do the trick.
Assignee

Updated

4 years ago
Keywords: checkin-needed
Assignee

Updated

4 years ago
Attachment #8677483 - Attachment description: Patch v2 → Patch v2 (Part 1)
Assignee

Comment 18

4 years ago
Just to make sure it fixed the Windows build error: https://treeherder.mozilla.org/#/jobs?repo=try&revision=55b32bfc208f
Backed out for suspicious-looking mochitest-bc failures on OSX 10.10.
https://treeherder.mozilla.org/logviewer.html#?job_id=5308048&repo=fx-team
Assignee

Comment 22

4 years ago
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #20)
> Backed out for suspicious-looking mochitest-bc failures on OSX 10.10.
> https://treeherder.mozilla.org/logviewer.html#?job_id=5308048&repo=fx-team

Huh, I can't reproduce it locally (on OSX 10.11, but not sure if there's much difference from OSX 10.10), even after multiple runs.
Confirmed green on the backout.
https://treeherder.mozilla.org/#/jobs?repo=fx-team&group_state=expanded&filter-searchStr=10.10%20bc1&fromchange=ccb7bb578fb1&tochange=d3061ac6aff9

If the try run goes green, I'd suspect a needs-clobber situation. Our build system is pretty finicky when file moves are involved.
Assignee

Comment 27

4 years ago
Attachment #8677483 - Attachment is obsolete: true
Attachment #8680323 - Flags: review+
Assignee

Comment 28

4 years ago
As you said on IRC, the new icon height caused the test failure. The test now makes sure to scroll the button into view (also replaced 2 other occurances of button.focus() that may cause trouble as well in the future).

Thanks for your help !
Attachment #8680326 - Flags: review?(dtownsend)
Attachment #8680326 - Flags: review?(dtownsend) → review+

Comment 31

4 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1083a388df14
https://hg.mozilla.org/mozilla-central/rev/04343ff95dfb
https://hg.mozilla.org/mozilla-central/rev/5b80b7408e4c
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion
Depends on: 1302725
You need to log in before you can comment on or make changes to this bug.