770433 - Update notification toast design

Status: RESOLVED FIXED

(Toolkit :: Themes, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

For the Social API work we're probably going to use the notification toast message to indicate a new chat message.

It'd be nice if we could update the design of the toast messages. This is an opportunity for us to incorporate some of the Australis design decisions, such as the slight border-radius and lighter colors. 

Stephen, can you put together a mockup of what you think Australis toast notifications should look like?

Comment 1

[u428464]

Some toasts/growls should also be suppressed (e.g. update notification and download completed one).

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

I'll see what I can do here.

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Updated border and background

This patch updates the border to 1px width with a 3px rounded border at the top-left and top-right and uses a similar background gradient as the newer arrow panels.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Video showing newer notification design

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Simple add-on for testing notification alerts

I created this simple add-on to test the notification alerts. To use this:
1) install the add-on
2) customize your toolbar by adding the Notification Tester button to your toolbar
3) click on the button to see a notification

Comment 6

[Justin Dolske [:Dolske]]

(In reply to Jared Wein [:jaws] from comment #5)
> Created attachment 643572 [details]
> Simple add-on for testing notification alerts

Or enable devtools.chrome.enabled, and cut'n'paste a small 5-liner. ;-)

Does the scope of this (or a separate) bug need to be widened, to include the ability to do more complex toast content? [Specifically wrt Social API stuff.] Or is icon + bold text + plain text a sufficient format?

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

I think the scope of this bug should remain small. We're only cleaning up the notifications on Windows (which is the only place that is supported and ugly).

As it stands now, I think the capabilities of the notification API are sufficient for Social API work.

Comment 8

[Stephen Horlander [:shorlander]]

Comment on attachment 643571 [details]
Video showing newer notification design

Looks like a nice improvement, thanks! :)

Comment 9

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 643570 [details] [diff] [review]
Updated border and background

>diff --git a/toolkit/themes/winstripe/global/alerts/alert.css b/toolkit/themes/winstripe/global/alerts/alert.css

> .alertBox {
>+  background-color: #f1f5fb;

Is there no system color that makes sense for this?

Dao would probably be a better reviewer for this kind of thing, but this is simple enough that I don't mind r+ing it!

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

I wish there was a system color for this, but it's the same color that is used for our arrow panels.

Pushed to mozilla-inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/77a086a60d01

Comment 11

[Dão Gottwald [:dao]]

Please file bugs in the right component such that component watching works.

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #9)
> > .alertBox {
> >+  background-color: #f1f5fb;
> 
> Is there no system color that makes sense for this?

You were on the right track, the hardcoded color is actually bogus here. It won't just look out of place but actually produce unreadable text depending on the OS theme.

(In reply to Jared Wein [:jaws] from comment #10)
> I wish there was a system color for this, but it's the same color that is
> used for our arrow panels.

Arrow panels use -moz-dialog.

Comment 12

[Dão Gottwald [:dao]]

Comment on attachment 643570 [details] [diff] [review]
Updated border and background

> .alertBox {

>+  border-bottom: none;

>+  border-top-left-radius: 3px;
>+  border-top-right-radius: 3px;

Also, notifications don't always slide from the bottom; depends on the task bar's position.

Comment 13

[Dão Gottwald [:dao]]

backout: http://hg.mozilla.org/integration/mozilla-inbound/rev/597ba95c6424

Comment 14

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch v2

Thanks for catching those issues and sorry for not requesting review from you on the earlier patch.

This patch keeps the rounded corners for the 4 various positions that the toast can appear from, and also uses -moz-dialog for the background-color.

Comment 15

[Dão Gottwald [:dao]]

Comment on attachment 645153 [details] [diff] [review]
Patch v2

>+  background-image: -moz-linear-gradient(top, white 1px, rgba(255,255,255,0) 15px);

This is unlikely to look decent for arbitrary backgrounds, so I think you'll want to limit it to the default theme.

Comment 16

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch v3

Oh great! I had forgot to ask what you thought about that so thanks for catching it.

Comment 17

[Dão Gottwald [:dao]]

Comment on attachment 645158 [details] [diff] [review]
Patch v3

>--- a/toolkit/components/alerts/resources/content/alert.js
>+++ b/toolkit/components/alerts/resources/content/alert.js
>@@ -83,16 +83,27 @@ function onAlertLoad()
>   else
>   {
>     if (gOrigin & NS_ALERT_TOP)
>       document.documentElement.pack = "end";
>   }
> 
>   var alertBox = document.getElementById("alertBox");
>   alertBox.orient = (gOrigin & NS_ALERT_HORIZONTAL) ? "vertical" : "horizontal";
>+  if (gOrigin & NS_ALERT_HORIZONTAL) {
>+    if (gOrigin & NS_ALERT_LEFT)
>+      alertBox.setAttribute("origin", "left");
>+    else
>+      alertBox.setAttribute("origin", "right");
>+  } else {
>+    if (gOrigin & NS_ALERT_TOP)
>+      alertBox.setAttribute("origin", "top");
>+    else
>+      alertBox.setAttribute("origin", "bottom");
>+  }

There's a similarly nested if/else block a few lines further up where you can incorporate this. You just need to declare alertBox earlier.

> .alertBox {
>-  border: 2px solid #7B969C;
>-  background-color: -moz-Dialog;
>+  border: 1px solid threedshadow;
>+  background-color: -moz-dialog;

nit: keep the case such that you're not needlessly replacing lines

>+@media (-moz-windows-default-theme) {
>+  .alertBox {
>+    background-image: -moz-linear-gradient(top, white 1px, rgba(255,255,255,0) 15px);
>+  }
>+}

linear-gradient has been unprefixed, you should use the new syntax.

Comment 18

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch v4

Thanks, feedback addressed.

Comment 19

[Dão Gottwald [:dao]]

Comment on attachment 645348 [details] [diff] [review]
Patch v4

Border and background styling looks good now.

I noticed some inconsistencies coming from the padding added to alertBox. With smaller icons, the notification height and visual padding is increased since alertImageBox has a min-height. alertTextBox also still has a padding as if the notification itself had none. I don't know if this was intended, but it seems like you're trying to center the text vertically, which should be done differently if actually wanted; either way the alertTextBox padding should be removed.

Comment 20

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch v5

I'm not sure if this is what you asked for, but I tested it out and it still looks good.

Comment 21

[Dão Gottwald [:dao]]

Comment on attachment 645576 [details] [diff] [review]
Patch v5

It looks like you missed the min-height part of comment 19...

Comment 22

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch v6

Let me know if this fixes the issue with min-height that you were talking about. I tested this out and made sure that there is a consistent 8px margin throughout the toast.

Comment 23

[Dão Gottwald [:dao]]

Comment on attachment 645819 [details] [diff] [review]
Patch v6

I don't understand what this patch is trying to do, the previous version seemed closer. You just need to adjust or remove the min-height of alertImageBox.

Comment 24

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch v7

Sorry, but I'm having a hard time understanding what you had asked for earlier.

In this patch, I removed the min-height for the horizontal alertImageBox, and the min-width for the vertical alertImageBox. I'm not sure why I would only remove the min-height for one but leave the min-width for the other.

Comment 25

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ea6eb14209b1

Comment 26

[(no longer active)]

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