Meta Bug for project https://github.com/Yoric/Mozilla-Student-Projects/issues/57 The Reader mode button, as implemented in https://bugzilla.mozilla.org/show_bug.cgi?id=558882, should enable the reader mode on a page when clicked. The button is visible only when the page is supported by Readability for parsing, otherwise it remains hidden.
Yuan and Shorlander, please feel free to chime in if you want to give any UX/design feedback for this student project. In general I think we can follow the lead of Firefox for Android, and Microsoft's design and typography guidelines.
Changes: Added Sprites for Reader mode icon. Added Reader mode icon to the left of the reload button. Clicking the Reader mode button logs a string message to the browser console. There is just one issue, when the browser is snapped to the smallest size (in immersive mode), there is a lot of gap between the reader mode icon and the reload icon.
(In reply to eklavyamirani from comment #2) > Created attachment 808538 [details] [diff] [review] > Added the reader mode icon to the left of the reload icon in urlbar > > Changes: > Added Sprites for Reader mode icon. > Added Reader mode icon to the left of the reload button. > Clicking the Reader mode button logs a string message to the browser console. > > There is just one issue, when the browser is snapped to the smallest size > (in immersive mode), there is a lot of gap between the reader mode icon and > the reload icon. Hi eklavyamirani, Could you please upload a screenshots of the reader mode icon and any different state you have? So I can make comments on your review request. Thanks!
Hi Yuan, As requested, here are the screenshots and the sprites that I created. I am using Windows Server 2012, which allows me to snap the app to custom sizes, and in some sizes the urlbar becomes inconsistent as you can see in the screenshots. Time to create a new bug? :) Thanks, Eklavya
Comment on attachment 808538 [details] [diff] [review] Added the reader mode icon to the left of the reload icon in urlbar Review of attachment 808538 [details] [diff] [review]: ----------------------------------------------------------------- The code looks fine with some trivial changes (see below). Thanks! r=mbrubeck, but this is not ready to land until it's hooked up to the actual about:reader code. >--- a/browser/metro/base/content/appbar.js This part of the patch file is malformed. Try running "hg qref -e" to get rid of it. ::: browser/metro/base/content/browser.xul @@ +260,5 @@ > <observes element="bcast_urlbarState" attribute="*"/> > <hbox id="toolbar-context-page" pack="end"> > <circularprogressindicator id="download-progress" > oncommand="Appbar.onDownloadButton()"/> > + Minor nit: This patch contains some unnecessary whitespace changes.
Attachment #810105 - Flags: review?(mbrubeck) → review+
Comment on attachment 808730 [details] Screenshots for reader mode. The icon and the states look fine with me
Attachment #808730 - Flags: ui-review?(ywang) → ui-review+
Patch is ready to land, but landing appears to be blocked by bug 795968 and other work owned by the desktop team.
(In reply to Yuan Wang(:Yuan) – Firefox UX Team from comment #7) > Comment on attachment 808730 [details] > Screenshots for reader mode. > > The icon and the states look fine with me These upscaled toolbar buttons images look awful. I think we need to get ux to scale these up properly.
Jim, please find the updated reader mode spec here: http://people.mozilla.org/~mmaslaney/readermode/index.html
This patch has been folded into bug 927124.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 927124
You need to log in before you can comment on or make changes to this bug.