Closed
Bug 634173
Opened 13 years ago
Closed 13 years ago
Style toaster alerts on Android
Categories
(Firefox for Android Graveyard :: General, defect)
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)
43.16 KB,
image/png
|
Details | |
7.06 KB,
patch
|
mbrubeck
:
review+
|
Details | Diff | Splinter Review |
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.
Updated•13 years ago
|
OS: Linux → Android
Hardware: x86_64 → All
Assignee | ||
Updated•13 years ago
|
tracking-fennec: --- → ?
Updated•13 years ago
|
tracking-fennec: ? → 2.0+
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → mark.finkle
Assignee | ||
Comment 1•13 years ago
|
||
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.
Assignee | ||
Comment 2•13 years ago
|
||
This shows the centering issue and the current CSS style on Android
Assignee | ||
Comment 3•13 years ago
|
||
Also, I left the image (icon) in the alert. We could remove that too. Thoughts?
Comment 4•13 years ago
|
||
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?
Assignee | ||
Comment 5•13 years ago
|
||
(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.
Assignee | ||
Comment 6•13 years ago
|
||
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 7•13 years ago
|
||
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+
Assignee | ||
Comment 8•13 years ago
|
||
pushed: http://hg.mozilla.org/mobile-browser/rev/683d3462a8ef
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•13 years ago
|
Flags: in-litmus?
Comment 9•13 years ago
|
||
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
Comment 10•13 years ago
|
||
https://litmus.mozilla.org/show_test.cgi?id=15207
Flags: in-litmus? → in-litmus+
You need to log in
before you can comment on or make changes to this bug.
Description
•