Closed Bug 634173 Opened 9 years ago Closed 9 years ago

Style toaster alerts on Android

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set

Tracking

(fennec2.0+)

VERIFIED FIXED
Tracking Status
fennec 2.0+ ---

People

(Reporter: mfinkle, Assigned: mfinkle)

References

Details

(Keywords: polish)

Attachments

(2 files, 1 obsolete file)

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.
OS: Linux → Android
Hardware: x86_64 → All
tracking-fennec: --- → ?
tracking-fennec: ? → 2.0+
Assignee: nobody → mark.finkle
Attached patch wip (obsolete) — Splinter Review
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.
Attached image android screenshot
This shows the centering issue and the current CSS style on Android
Also, I left the image (icon) in the alert. We could remove that too. Thoughts?
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?
(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.
Attached patch patchSplinter Review
Same as previous patch, but I removed the shadow above the toaster alert
Attachment #513885 - Attachment is obsolete: true
Attachment #514502 - Flags: review?(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;
Attachment #514502 - Flags: review?(mbrubeck) → review+
pushed:
http://hg.mozilla.org/mobile-browser/rev/683d3462a8ef
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Flags: in-litmus?
verified fixed in build id: Mozilla /5.0 (Android;Linux armv7l;rv:2.0b13pre) Gecko/20110307 Firefox/4.0b13pre Fennec /4.0b6pre
Status: RESOLVED → VERIFIED
https://litmus.mozilla.org/show_test.cgi?id=15207
Flags: in-litmus? → in-litmus+
Depends on: 643301
You need to log in before you can comment on or make changes to this bug.