Add Reader Mode Icon to the URL bar of Metro Firefox

RESOLVED DUPLICATE of bug 927124

Status

enhancement
P1
normal
RESOLVED DUPLICATE of bug 927124
6 years ago
5 years ago

People

(Reporter: eklavyamirani, Assigned: eklavyamirani)

Tracking

({feature})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Assignee

Description

6 years ago
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.
Assignee: nobody → eklavyamirani
Status: UNCONFIRMED → ASSIGNED
Depends on: desktop-reader
Ever confirmed: true
Keywords: feature, uiwanted
OS: Windows 8 → Windows 8 Metro
Hardware: x86_64 → All
Assignee

Comment 2

6 years ago
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.
Attachment #808538 - Flags: ui-review?(ywang)
Attachment #808538 - Flags: review?(mbrubeck)
(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!
Assignee

Comment 4

6 years ago
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
Attachment #808730 - Flags: ui-review?(ywang)
Attachment #808730 - Flags: review?(mbrubeck)
Attachment #808730 - Attachment mime type: application/octet-stream → application/zip
Attachment #808730 - Flags: review?(mbrubeck)
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 #808538 - Flags: ui-review?(ywang)
Attachment #808538 - Flags: review?(mbrubeck)
Attachment #808538 - Flags: review+
Assignee

Updated

6 years ago
See Also: → 920712
Assignee

Comment 6

6 years ago
Attachment #808538 - Attachment is obsolete: true
Attachment #810105 - Flags: review?(mbrubeck)
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+
Assignee

Updated

6 years ago
Blocks: 927124
Assignee

Updated

6 years ago
See Also: → 927124
No longer blocks: metrov2planning
Whiteboard: [feature] p=0
Assignee

Updated

6 years ago
No longer depends on: desktop-reader
Patch is ready to land, but landing appears to be blocked by bug 795968 and other work owned by the desktop team.
Priority: P3 → --
Priority: -- → P1
Keywords: uiwanted
Whiteboard: [feature] p=0 → p=0
(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.
Flags: needinfo?(mmaslaney)
Jim, please find the updated reader mode spec here:


http://people.mozilla.org/~mmaslaney/readermode/index.html
Flags: needinfo?(mmaslaney)
This patch has been folded into bug 927124.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 927124
No longer blocks: metrobacklog, 927124
Whiteboard: p=0
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.