Update notification toast design

RESOLVED FIXED in mozilla17

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jaws, Assigned: jaws)

Tracking

Trunk
mozilla17
All
Windows XP
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 6 obsolete attachments)

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?
Whiteboard: [Fx16] → [Fx17]

Comment 1

6 years ago
Some toasts/growls should also be suppressed (e.g. update notification and download completed one).
Whiteboard: [Fx17] → [Fx16]
I'll see what I can do here.
Assignee: shorlander → jaws
Status: NEW → ASSIGNED
Created attachment 643570 [details] [diff] [review]
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.
Created attachment 643571 [details]
Video showing newer notification design
Attachment #643571 - Flags: ui-review?(shorlander)
Created attachment 643572 [details]
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
(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?
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.
OS: All → Windows XP
Comment on attachment 643571 [details]
Video showing newer notification design

Looks like a nice improvement, thanks! :)
Attachment #643571 - Flags: ui-review?(shorlander) → ui-review+
Attachment #643570 - Flags: review?(gavin.sharp)
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!
Attachment #643570 - Flags: review?(gavin.sharp) → review+
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
Please file bugs in the right component such that component watching works.

(In reply to :Gavin Sharp (use gavin@gavinsharp.com 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.
Component: General → Themes
Product: Firefox → Toolkit
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.
Attachment #643570 - Flags: review-
Created attachment 645153 [details] [diff] [review]
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.
Attachment #643570 - Attachment is obsolete: true
Attachment #645153 - Flags: review?(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.
Created attachment 645158 [details] [diff] [review]
Patch v3

Oh great! I had forgot to ask what you thought about that so thanks for catching it.
Attachment #645153 - Attachment is obsolete: true
Attachment #645153 - Flags: review?(dao)
Attachment #645158 - Flags: review?(dao)
Attachment #645158 - Flags: review?(gavin.sharp)
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.
Created attachment 645348 [details] [diff] [review]
Patch v4

Thanks, feedback addressed.
Attachment #645158 - Attachment is obsolete: true
Attachment #645158 - Flags: review?(gavin.sharp)
Attachment #645158 - Flags: review?(dao)
Attachment #645348 - Flags: review?(gavin.sharp)
Attachment #645348 - Flags: review?(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.
Attachment #645348 - Flags: review?(dao) → review-
Created attachment 645576 [details] [diff] [review]
Patch v5

I'm not sure if this is what you asked for, but I tested it out and it still looks good.
Attachment #645348 - Attachment is obsolete: true
Attachment #645348 - Flags: review?(gavin.sharp)
Attachment #645576 - Flags: review?(dao)

Updated

6 years ago
Attachment #645576 - Flags: review?(dao) → review+
Comment on attachment 645576 [details] [diff] [review]
Patch v5

It looks like you missed the min-height part of comment 19...
Attachment #645576 - Flags: review+
Created attachment 645819 [details] [diff] [review]
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.
Attachment #645576 - Attachment is obsolete: true
Attachment #645819 - Flags: review?(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.
Attachment #645819 - Flags: review?(dao) → review-
Created attachment 645894 [details] [diff] [review]
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.
Attachment #645819 - Attachment is obsolete: true
Attachment #645894 - Flags: review?(dao)

Updated

6 years ago
Attachment #645894 - Flags: review?(dao) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/ea6eb14209b1
Flags: in-testsuite-
Target Milestone: --- → mozilla17

Comment 26

6 years ago
https://hg.mozilla.org/mozilla-central/rev/ea6eb14209b1
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [Fx16]
You need to log in before you can comment on or make changes to this bug.