634173 - Style toaster alerts on Android

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

The current style doesn't fit the Android native toaster look very well at all. Also, native Android toasters appear in the lower middle of the screen. Currently we slide in from the lower right corner, and stay in the lower right corner.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: wip

This patch updates the alerts for desktop and Android in the following ways:
* CSS theme is now fixed so the alerts look as they used too: dark and appropriate padding
* Android toaster alerts are now centered (or mostly centered)
* The toaster alerts size themselves to fit the screen better. Previously they were hardcoded to 300px. Now they grow to fit the content, limited by the window width. If too large for the window, we set a max width and allow the text to wrap.

In doing the fit-to-content part, I found the transition affect used on Maemo and desktop a PITA to get working correctly, so I used the Android fade-in / fade-out for it too.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: android screenshot

This shows the centering issue and the current CSS style on Android

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Also, I left the image (icon) in the alert. We could remove that too. Thoughts?

Comment 4

[Madhava Enros [:madhava]]

I'd leave the icon in.

At first I thought the gray background color was wrong, but it checks out compared to our other background gray:
http://www.flickr.com/photos/madhava_work/5471403834/

We probably want to revisit our overall background color in the next release.

Two minor changes:

1. I don't think there should be a shadow along the top of the toaster, just the bottom
2. Android native ones are wider -- i.e. they occupy more of the width of the screen.  I'm not sure how we're sizing these right now (do they stretch to fit the content?) - but if they're just a set width, maybe we can increase it so that it's about 2/3 of the screen width?

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #4)
> I'd leave the icon in.

OK

> 1. I don't think there should be a shadow along the top of the toaster, just
> the bottom

I'll fix that

> 2. Android native ones are wider -- i.e. they occupy more of the width of the
> screen.  I'm not sure how we're sizing these right now (do they stretch to fit
> the content?) - but if they're just a set width, maybe we can increase it so
> that it's about 2/3 of the screen width?

We size the toaster based on the content and only fiddle with the width if the toaster will be too big for screen. Then we kick in text wrapping and limit the width, growing vertically.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch

Same as previous patch, but I removed the shadow above the toaster alert

Comment 7

[Matt Brubeck (:mbrubeck)]

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

># HG changeset patch
># Parent ef94447b908edc08711fdbf940719fa1fc4baebd
>
>diff --git a/chrome/content/AlertsHelper.js b/chrome/content/AlertsHelper.js
>--- a/chrome/content/AlertsHelper.js
>+++ b/chrome/content/AlertsHelper.js
>@@ -4,41 +4,63 @@ var AlertsHelper = {
>   _cookie: "",
>   _clickable: false,
>   get container() {
>     delete this.container;
>     let container = document.getElementById("alerts-container");
> 
>     // Move the popup on the other side if we are in RTL
>     let [leftSidebar, rightSidebar] = [Elements.tabs.getBoundingClientRect(), Elements.controls.getBoundingClientRect()];
>-    if (leftSidebar.left > rightSidebar.left) {
>-      container.removeAttribute("right");
>+    if (leftSidebar.left > rightSidebar.left)
>       container.setAttribute("left", "0");
>-    }
>+    else
>+      container.setAttribute("right", "0");
> 
>     let self = this;
>     container.addEventListener("transitionend", function() {
>       self.alertTransitionOver();
>     }, true);
> 
>     return this.container = container;
>   },
> 
>   showAlertNotification: function ah_show(aImageURL, aTitle, aText, aTextClickable, aCookie, aListener) {
>     this._clickable = aTextClickable || false;
>     this._listener = aListener || null;
>     this._cookie = aCookie || "";
> 
>+    // Reset the container settings from the last time so layout can happen naturally
>+    let container = this.container;
>+    container.removeAttribute("width");
>+    let alertText = document.getElementById("alerts-text");
>+    alertText.style.whiteSpace = "";
>+
>     document.getElementById("alerts-image").setAttribute("src", aImageURL);
>     document.getElementById("alerts-title").value = aTitle;
>-    document.getElementById("alerts-text").textContent = aText;
>+    alertText.textContent = aText;
> 
>-    let container = this.container;
>     container.hidden = false;
>-    container.height = container.getBoundingClientRect().height;
>+    let bcr = container.getBoundingClientRect();
>+    if (bcr.width > window.innerWidth - 50) {
>+      // If the window isn't wide enough, we need to re-layout
>+      container.setAttribute("width", window.innerWidth - 50); // force a max width
>+      alertText.style.whiteSpace = "pre-wrap"; // wrap text as needed
>+      bcr = container.getBoundingClientRect(); // recalculate the bcr
>+    }
>+    container.setAttribute("width", bcr.width); // redundant but cheap
>+    container.setAttribute("height", bcr.height);
>+
>+#ifdef ANDROID
>+    let offset = (window.innerWidth - container.width) / 2;
>+    if (container.hasAttribute("left"))
>+      container.setAttribute("left", offset);
>+    else
>+      container.setAttribute("right", offset);
>+#endif
>+
>     container.classList.add("showing");
> 
>     let timeout = Services.prefs.getIntPref("alerts.totalOpenTime");
>     let self = this;
>     if (this._timeoutID)
>       clearTimeout(this._timeoutID);
>     this._timeoutID = setTimeout(function() { self._timeoutAlert(); }, timeout);
>   },
>diff --git a/chrome/content/browser.xul b/chrome/content/browser.xul
>--- a/chrome/content/browser.xul
>+++ b/chrome/content/browser.xul
>@@ -656,18 +656,17 @@
>     <hbox id="menulist-container" class="window-width window-height context-block" top="0" left="0" hidden="true" flex="1">
>       <vbox id="menulist-popup" class="dialog-dark">
>         <label id="menulist-title" crop="center" flex="1"/>
>         <richlistbox id="menulist-commands" class="prompt-buttons" onclick="if (event.target != this) MenuListHelperUI.selectByIndex(this.selectedIndex);" flex="1"/>
>       </vbox>
>     </hbox>
> 
>     <!-- alerts for content -->
>-    <hbox id="alerts-container" hidden="true" align="start" class="dialog-dark" bottom="0" right="0"
>-          onclick="AlertsHelper.click(event);">
>+    <hbox id="alerts-container" hidden="true" align="start" bottom="0" onclick="AlertsHelper.click(event);">
>       <image id="alerts-image"/>
>       <vbox flex="1">
>         <label id="alerts-title" value=""/>
>         <description id="alerts-text" flex="1"/>
>       </vbox>
>     </hbox>
> 
>     <hbox id="appmenu" bottom="0" hidden="true" align="stretch" oncommand="AppMenu.hide();">
>diff --git a/chrome/jar.mn b/chrome/jar.mn
>--- a/chrome/jar.mn
>+++ b/chrome/jar.mn
>@@ -17,17 +17,17 @@ chrome.jar:
>   content/aboutHome.xhtml              (content/aboutHome.xhtml)
> * content/aboutRights.xhtml            (content/aboutRights.xhtml)
>   content/languages.properties         (content/languages.properties)
> * content/browser.xul                  (content/browser.xul)
> * content/browser.js                   (content/browser.js)
> * content/browser-ui.js                (content/browser-ui.js)
> * content/browser-scripts.js           (content/browser-scripts.js)
> * content/common-ui.js                 (content/common-ui.js)
>-  content/AlertsHelper.js              (content/AlertsHelper.js)
>+* content/AlertsHelper.js              (content/AlertsHelper.js)
>   content/AppMenu.js                   (content/AppMenu.js)
>   content/AwesomePanel.js              (content/AwesomePanel.js)
>   content/BookmarkHelper.js            (content/BookmarkHelper.js)
>   content/BookmarkPopup.js             (content/BookmarkPopup.js)
> * content/ContextCommands.js           (content/ContextCommands.js)
>   content/MenuListHelperUI.js          (content/MenuListHelperUI.js)
>   content/OfflineApps.js               (content/OfflineApps.js)
>   content/SelectHelperUI.js            (content/SelectHelperUI.js)
>diff --git a/themes/core/browser.css b/themes/core/browser.css
>--- a/themes/core/browser.css
>+++ b/themes/core/browser.css
>@@ -1267,46 +1267,47 @@ pageaction:not([image]) > hbox >.pageact
> /* XXX should be a richlistitem description.normal */
> .prefdesc {
>   font-size: @font_small@ !important;
>   color: grey;
> }
> 
> /* alerts popup ----------------------------------------------------------- */
> #alerts-container {
>-  width: 300px;
>-}
>-
>-@media (max-width: 499px) {
>-  #alerts-container {
>-    width: 200px;
>-  }
>-}
>-
>-#alerts-container {
>-  -moz-transform: translatex(100%);
>-  -moz-transition-property: -moz-transform;
>+  color: white;
>+  background-color: #5e6166;
>+  border: @border_width_small@ solid #767973;
>+  border-radius: @border_radius_normal@;
>+  box-shadow: black 0 @border_radius_tiny@ @border_radius_tiny@;
>+  padding: @padding_normal@; /* core spacing on top/bottom */
>+  margin-bottom: @margin_large@;
>+  -moz-transition-property: opacity;
>   -moz-transition-duration: 0.5s;
>-  margin-bottom: @margin_large@;
>-  -moz-margin-end: @margin_large@;
>-}
>-
>-#alerts-container:-moz-locale-dir(rtl) {
>-  -moz-transform: translatex(-100%);
>+  opacity: 0;
> }
> 
> #alerts-container.showing {
>-  -moz-transform: translatex(0);
>+  opacity: 1;
>+}
>+
>+#alerts-title {
>+  font-size: @font_small@ !important;
> }
> 
> #alerts-text {
>-  font-size: @font_small@ !important;
>-  white-space: pre-wrap;
>+  font-size: @font_xsmall@ !important;
>+  white-space: pre;
> }
> 
>+%ifndef ANDROID
>+#alerts-container {
>+  -moz-margin-end: @margin_large@;
>+}
>+%endif
>+
> /* helperapp (save-as) popup ----------------------------------------------- */
> #helperapp-target {
>   font-size: @font_small@ !important;
> }
> 
> /* navigator popup -------------------------------------------------------------- */
> #content-navigator {
>   padding: 0;

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed:
http://hg.mozilla.org/mobile-browser/rev/683d3462a8ef

Comment 9

[Ioana Chiorean]

verified fixed in build id: Mozilla /5.0 (Android;Linux armv7l;rv:2.0b13pre) Gecko/20110307 Firefox/4.0b13pre Fennec /4.0b6pre

Comment 10

[Ioana Chiorean]

https://litmus.mozilla.org/show_test.cgi?id=15207