Last Comment Bug 818017 - Sync changes in global/alerts/alert.css from Toolkit (Adjust styling of nsIAlertService alert windows).
: Sync changes in global/alerts/alert.css from Toolkit (Adjust styling of nsIAl...
Status: RESOLVED FIXED
: modern
Product: SeaMonkey
Classification: Client Software
Component: Themes (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.17
Assigned To: Philip Chee
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-12-04 05:54 PST by Philip Chee
Modified: 2012-12-06 07:46 PST (History)
0 users
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v1.0 Update. (2.14 KB, patch)
2012-12-04 06:00 PST, Philip Chee
neil: review-
Details | Diff | Splinter Review
Patch v1.1 fix nits. (2.11 KB, patch)
2012-12-05 09:37 PST, Philip Chee
neil: review+
Details | Diff | Splinter Review

Description Philip Chee 2012-12-04 05:54:50 PST
Rollup of changes from:
Bug 769095. Enforce maximum icon size for nsIAlertsService's notification.
Bug 770433 - Update borders and background of alert (toast) notifications on Windows.
Bug 786125 - Alert showing/hiding animation is janky (nsIAlertsService).
Bug 796111. Adjust styling of nsIAlertService alert windows.

See: http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/winstripe/global/alerts/alert.css
Comment 1 Philip Chee 2012-12-04 06:00:54 PST
Created attachment 688230 [details] [diff] [review]
Patch v1.0 Update.

> Bug 786125 - Alert showing/hiding animation is janky (nsIAlertsService)
So now instead of the toaster sliding in, it fades in using opacity+animation.

> See: http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/winstripe/global/alerts/alert.css

I didn't bother to implement the background-image: linear-gradient(...) bits.

To test use:

Classic:
Components.classes["@mozilla.org/alerts-service;1"]
          .getService(Components.interfaces.nsIAlertsService)
          .showAlertNotification("chrome://global/skin/icons/information-48.png",
                                 "Notification test",
                                 "Surprise! I'm here to test notifications!",
                                 true, "foobarcookie", null);


Modern:
Components.classes["@mozilla.org/alerts-service;1"]
          .getService(Components.interfaces.nsIAlertsService)
          .showAlertNotification("chrome://global/skin/icons/alert-message.gif",
                                 "Notification test",
                                 "Surprise! I'm here to test notifications!",
                                 true, "foobarcookie", null);
Comment 2 neil@parkwaycc.co.uk 2012-12-04 09:03:57 PST
(In reply to Philip Chee from comment #1)
> So now instead of the toaster sliding in, it fades in using opacity+animation.
Unfortunately that doesn't seem to work on Remote Desktop...

> I didn't bother to implement the background-image: linear-gradient(...) bits.
No, that would probably have been silly.
Comment 3 neil@parkwaycc.co.uk 2012-12-04 09:07:38 PST
(In reply to comment #2)
> (In reply to Philip Chee from comment #1)
> > So now instead of the toaster sliding in, it fades in using opacity+animation.
> Unfortunately that doesn't seem to work on Remote Desktop...
I tried it in Classic on another PC and it doesn't actually fade in - it appears, then the content fades in and out, then it disappears...
Comment 4 neil@parkwaycc.co.uk 2012-12-04 09:21:11 PST
Comment on attachment 688230 [details] [diff] [review]
Patch v1.0 Update.

>+.alertImageBox {
>+  padding: 8px 0;
It seems that padding: 8px; suffices.
(Modern style is to use 0px instead of just 0.)

>+  -moz-padding-start: 4px;
Not sure what good this does.

> .alertText {
>   -moz-margin-end: 6px;
> }
Probably superfluous now.

> .alertText[clickable="true"] {
>   cursor: pointer;
As is this.

>+@keyframes alert-animation {
>+  from {
>+    opacity: 0;
>+  }
>+  6.25% {
>+    opacity: 1;
>+  }
>+  93.75% {
>+    opacity: 1;
>+  }
>+  to {
>+    opacity: 0;
>+  }
>+}
I guess we need this for the alert to stand any chance of being eyecatching...
Comment 5 Philip Chee 2012-12-05 09:37:24 PST
Created attachment 688829 [details] [diff] [review]
Patch v1.1 fix nits.

>>+.alertImageBox {
>>+  padding: 8px 0;
> It seems that padding: 8px; suffices.
> (Modern style is to use 0px instead of just 0.)
Fixed.

>>+  -moz-padding-start: 4px;
> Not sure what good this does.
Removed.

>> .alertText {
>>   -moz-margin-end: 6px;
>> }
> Probably superfluous now.
Removed.

>> .alertText[clickable="true"] {
>>   cursor: pointer;
> As is this.
Removed.
Comment 6 Philip Chee 2012-12-06 07:46:47 PST
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/1fbd802f8ebd

Note You need to log in before you can comment on or make changes to this bug.