1162584 - Update install flow with new icons from bug 1144599

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Dave Townsend [:mossop]]

Once we have the new artwork we need to sweep through and update them in the tree.

Comment 1

[Dave Townsend [:mossop]]

Attachment: patch

This adds the new icons and updates the styles. For the install complete notification I've split it into two doorhangers to make styling the icon easier. For the confirmation doorhanger it's a little harder since it is a more complex doorhanger so I'm just adding a warning class where we need to show the warning icon.

Comment 2

[Dão Gottwald [:dao]]

Comment on attachment 8606010 [details] [diff] [review]
patch

>+      let notification = document.getElementById("addon-install-confirmation-notification");
>       if (unsigned.length == installInfo.installs.length) {
>         // None of the add-ons are verified
>         messageString = gNavigatorBundle.getString("addonConfirmInstallUnsigned.message");
>+        notification.classList.add("warning");
>       }
>       else if (unsigned.length == 0) {
>         // All add-ons are verified or don't need to be verified
>         messageString = gNavigatorBundle.getString("addonConfirmInstall.message");
>+        notification.classList.remove("warning");
>       }
>       else {
>         // Some of the add-ons are unverified, the list of names will indicate
>         // which
>         messageString = gNavigatorBundle.getString("addonConfirmInstallSomeUnsigned.message");
>+        notification.classList.add("warning");
>       }

>+#addon-install-confirmation-notification.warning .popup-notification-icon[popupid="addon-install-confirmation"] {
>+  list-style-image: url(chrome://browser/skin/addons/addons-warning.svg);
>+}

In our code base, classes are supposed to be unambiguous such that you don't get unintentionally affected by some other styles using that class. So please name this class distinctively or use an attribute (e.g. #addon-install-confirmation-notification[warning]).

>+  skin/classic/browser/addons/addons-blocked.svg            (../shared/addons/addons-blocked.svg)
>+  skin/classic/browser/addons/addons-confirm.svg            (../shared/addons/addons-confirm.svg)
>+  skin/classic/browser/addons/addons-downloading.svg        (../shared/addons/addons-downloading.svg)
>+  skin/classic/browser/addons/addons-error.svg              (../shared/addons/addons-error.svg)
>+  skin/classic/browser/addons/addons-installed.svg          (../shared/addons/addons-installed.svg)
>+  skin/classic/browser/addons/addons-restart.svg            (../shared/addons/addons-restart.svg)
>+  skin/classic/browser/addons/addons-warning.svg            (../shared/addons/addons-warning.svg)

addon-install-blocked.svg, addon-install-confirm.svg, addon-install-downloading.svg etc.

>+  skin/classic/browser/addons/notifications-addons.svg      (../shared/addons/notifications-addons.svg)

addon-install-anchor.svg

>+#addons-notification-icon:active {
>+  list-style-image: url(chrome://browser/skin/addons/notifications-addons.svg#active);
> }

should be :hover:active rather than :active

Comment 3

[Tim Nguyen :ntim]

Comment on attachment 8606010 [details] [diff] [review]
patch

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

A couple of comments about the SVGs. The comments on addons-blocked.svg apply to all SVG files.

::: browser/themes/shared/addons/addons-blocked.svg
@@ +1,2 @@
> +<?xml version="1.0" encoding="utf-8"?>
> +<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">

nit : This DOCTYPE can be removed.

@@ +6,5 @@
> +     xmlns:xlink="http://www.w3.org/1999/xlink"
> +     width="64"
> +     height="64"
> +     viewBox="0 0 64 64">
> +       

nit : Lots of trailing whitespace here and throughout the SVG files.

@@ +10,5 @@
> +       
> +  <defs>
> +
> +    <style type="text/css">
> +    <![CDATA[

nit : useless wrapping with CDATA

@@ +14,5 @@
> +    <![CDATA[
> +      .style-puzzle-piece {
> +        fill: url('#gradient-linear-puzzle-piece');
> +      }
> +      

nit : trailing whitespace

@@ +16,5 @@
> +        fill: url('#gradient-linear-puzzle-piece');
> +      }
> +      
> +      .style-badge-shadow {
> +        fill: #0d131a; 

nit : trailing whitespace

@@ +21,5 @@
> +        fill-opacity: .15;
> +      }
> +
> +      .style-badge-background {
> +        fill: #fff; 

nit : trailing whitespace

@@ +23,5 @@
> +
> +      .style-badge-background {
> +        fill: #fff; 
> +      }
> +      

nit : trailing whitespace

@@ +25,5 @@
> +        fill: #fff; 
> +      }
> +      
> +      .style-badge-inside {
> +        fill: #e62117; 

nit : trailing whitespace

@@ +27,5 @@
> +      
> +      .style-badge-inside {
> +        fill: #e62117; 
> +      }
> +      

nit : trailing whitespace

@@ +29,5 @@
> +        fill: #e62117; 
> +      }
> +      
> +      .style-badge-icon {
> +        fill: #fff; 

nit : trailing whitespace

@@ +31,5 @@
> +      
> +      .style-badge-icon {
> +        fill: #fff; 
> +      }
> +      

nit : trailing whitespace

@@ +49,5 @@
> +    <circle  class="style-badge-background" r="15"  cy="15" cx="16" />
> +    <circle  class="style-badge-inside"     r="12"  cy="15" cx="16" />
> +    <rect    class="style-badge-icon"       x="9" y="13" width="14" height="4" rx="1" ry="1" />
> +  </svg>
> +    

nit : trailing whitespace

::: browser/themes/shared/addons/addons-confirm.svg
@@ +6,5 @@
> +     xmlns:xlink="http://www.w3.org/1999/xlink"
> +     width="64"
> +     height="64"
> +     viewBox="0 0 64 64">
> +       

nit : trailing whitespace

@@ +10,5 @@
> +       
> +  <defs>
> +
> +    <style type="text/css">
> +    <![CDATA[

nit : useless wrapping with CDATA

@@ +14,5 @@
> +    <![CDATA[
> +      .style-puzzle-piece {
> +        fill: url('#gradient-linear-puzzle-piece');
> +      }
> +      

nit : trailing whitespace

@@ +26,5 @@
> +
> +  </defs>
> +
> +  <path id="puzzle-piece" class="style-puzzle-piece" d="M42,62c2.2,0,4-1.8,4-4l0-14.2c0,0,0.4-3.7,2.8-3.7c2.4,0,2.2,3.9,6.7,3.9c2.3,0,6.2-1.2,6.2-8.2 c0-7-3.9-7.9-6.2-7.9c-4.5,0-4.3,3.7-6.7,3.7c-2.4,0-2.8-3.8-2.8-3.8V22c0-2.2-1.8-4-4-4H31.5c0,0-3.4-0.6-3.4-3 c0-2.4,3.8-2.6,3.8-7.1c0-2.3-1.3-5.9-8.3-5.9s-8,3.6-8,5.9c0,4.5,3.4,4.7,3.4,7.1c0,2.4-3.4,3-3.4,3H6c-2.2,0-4,1.8-4,4l0,7.8 c0,0-0.4,6,4.4,6c3.1,0,3.2-4.1,7.3-4.1c2,0,4,1.9,4,6c0,4.2-2,6.3-4,6.3c-4,0-4.2-4.1-7.3-4.1c-4.8,0-4.4,5.8-4.4,5.8L2,58 c0,2.2,1.8,4,4,4H19c0,0,6.3,0.4,6.3-4.4c0-3.1-4-3.6-4-7.7c0-2,2.2-4.5,6.4-4.5c4.2,0,6.6,2.5,6.6,4.5c0,4-3.9,4.6-3.9,7.7 c0,4.9,6.3,4.4,6.3,4.4H42z"/>
> +    

nit : trailing whitespace

Comment 4

[Pulsebot]

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

Comment 5

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/a412f9c18d6b

Comment 6

[Dave Townsend [:mossop]]

Decided we won't uplift this to 40.

Comment 7

[Vasilica Tamas, QA [:vtamas]]

Verified fixed on Firefox 41.0a1 (2015-06-08) under Windows 7 64-bit, Ubuntu 14.04 32-bit and Mac OS X 10.9.5.

The testing was based on http://people.mozilla.org/~shorlander/mockups/Add-ons-Install-Flow/Add-ons-Install-Flow-Icons.html