Closed
Bug 998031
Opened 11 years ago
Closed 10 years ago
Reader Mode toolbar should scroll in and out instead of fading
Categories
(Firefox for Android Graveyard :: Reader View, defect)
Tracking
(firefox38 verified, firefox39 verified)
RESOLVED
FIXED
Firefox 39
People
(Reporter: ibarlow, Assigned: Margaret)
References
Details
Attachments
(2 files, 1 obsolete file)
We should adjust the Reader toolbar behaviour to slide off the screen when a user scrolls down a page, and reappear when a user scrolls back up.
This will make the interaction more consistent with our title bar behavour, and also make the interaction of revealing it somewhat more discoverable than it is.
Note: We will also want to remove the "tap screen to show toolbar" toast when we fix this bug.
Updated•11 years ago
|
Whiteboard: [mentor=lucasr][lang=js]
Updated•10 years ago
|
Mentor: lucasr.at.mozilla
Whiteboard: [mentor=lucasr][lang=js] → [lang=js]
Assignee | ||
Comment 2•10 years ago
|
||
(In reply to Michael Comella (:mcomella) from comment #1)
> Margaret, would you like to mentor?
This code is undergoing a lot of change right now, so I'm not sure this would be a good mentor bug right now.
antlam, is this something we would want?
Blocks: readerv2
Mentor: lucasr.at.mozilla → nobody
Flags: needinfo?(margaret.leibovic) → needinfo?(alam)
Comment 3•10 years ago
|
||
Yes! let's get this in. I think that slow fade-out and in is one of the things that makes this bar hard to discover.
Side note: the relationship between Reading List and Reader Mode (mentioned in bug 1103232) could benefit from this a bit too.
Flags: needinfo?(alam)
Comment 4•10 years ago
|
||
Partial dupe of Bug 1114708.
Comment 5•10 years ago
|
||
Attaching a quick mock up to just visually show what we're talking about here. I hacked the mock together quickly so we might have to tweak a couple things but it gets the point across... hopefully :)
Flags: needinfo?(margaret.leibovic)
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to Anthony Lam (:antlam) from comment #5)
> Created attachment 8560432 [details]
> readermode_scroll_anim1.mov
>
> Attaching a quick mock up to just visually show what we're talking about
> here. I hacked the mock together quickly so we might have to tweak a couple
> things but it gets the point across... hopefully :)
This makes sense, and it's consistent with our home banner UX. Fixing this will involve adding some gesture detection to AboutReader.jsm:
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/reader/AboutReader.jsm
While working on bug 1120735, I'm running into some annoying issues trying to share logic for the desktop/mobile controls, so I may just end up forking these to make it easier to maintain.
I think we should wait for that bug to land before starting on this, although we could start prototyping a solution before then.
Mentor: nobody → margaret.leibovic
Flags: needinfo?(margaret.leibovic)
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → margaret.leibovic
Assignee | ||
Updated•10 years ago
|
Mentor: margaret.leibovic
Whiteboard: [lang=js]
Assignee | ||
Comment 7•10 years ago
|
||
To clarify the desired behavior here, do we want this toolbar to appear any time you scroll up? So, as soon as the user starts scrolling down, we scroll the toolbar off screen at the same scroll rate, but as soon as they scroll up, we bring it back? I assume we wouldn't want to let them get into any state where the bar is partially showing.
It's unfortunate that we already have this logic implemented in Java for the home banner, but that won't be usable here :/
Flags: needinfo?(alam)
Assignee | ||
Comment 8•10 years ago
|
||
Here's a build to try out:
http://people.mozilla.org/~mleibovic/tmp/toolbar.apk
This just swaps out the fade animation with a vertical translation animation, so it doesn't do anything special to track the user's finger as they scroll.
I think this is an improvement over what we have now, but I'm happy to do more work to refine this before posting something for review.
The first time the bar appears, it looks a bit janky, but I think that's because of our fixed-position platform issues... I can try to see if there's a way to work around that, but we may not have much luck there :/
Comment 9•10 years ago
|
||
(In reply to :Margaret Leibovic from comment #7)
> To clarify the desired behavior here, do we want this toolbar to appear any
> time you scroll up? So, as soon as the user starts scrolling down, we scroll
> the toolbar off screen at the same scroll rate, but as soon as they scroll
> up, we bring it back?
Yes! Ideally, at the exact same rate.
> I assume we wouldn't want to let them get into any
> state where the bar is partially showing.
nope, never "stuck", just like our browser chrome.
(In reply to :Margaret Leibovic from comment #8)
> Here's a build to try out:
> http://people.mozilla.org/~mleibovic/tmp/toolbar.apk
>
> This just swaps out the fade animation with a vertical translation
> animation, so it doesn't do anything special to track the user's finger as
> they scroll.
The animation coming in on "press" is nice and smooth. I think we need to refine the animation when the user "scrolls up" (probably the "tracking" you're mentioning).
> I think this is an improvement over what we have now, but I'm happy to do
> more work to refine this before posting something for review.
>
> The first time the bar appears, it looks a bit janky, but I think that's
> because of our fixed-position platform issues... I can try to see if there's
> a way to work around that, but we may not have much luck there :/
Yeah, on load, it feels a bit like it's crashing in ATM. Hm...
Also, maybe outside the scope of this bug, but I noticed that the hit areas of +/- under the "Aa" can be triggered. I can hit it and it's sometimes +, sometimes -. We should just shrink that hit area so it doesn't extend to the "Aa" in the middle.
Flags: needinfo?(alam)
Assignee | ||
Comment 10•10 years ago
|
||
/r/5231 - Bug 998031 - Reader Mode toolbar should scroll in and out instead of fading. r=bnicholson
Pull down this commit:
hg pull review -r 510394506056ecca9e4db6b6364c3bb68892d8fa
Attachment #8576400 -
Flags: review?(bnicholson)
Assignee | ||
Comment 11•10 years ago
|
||
I tried hard to get the toolbar to scroll off screen at the same rate at the user's finger, but this seems just not possible with our current touch event support (bug 1142286).
I don't think I can afford to spend more time digging into this, so I'd like to just land this transition-only change, since it's definitely an improvement over the fade.
To address the jankiness when the toolbar first appears (which is an issue with the current implementation as well), I decided to just add a setTimeout around showing the toolbar. I think there's some underlaying issue here with position: fixed; elements, but this appears to fix the issue, and I'm ok with a band-aid in this case.
Comment 12•10 years ago
|
||
Comment on attachment 8576400 [details]
MozReview Request: bz://998031/margaret
https://reviewboard.mozilla.org/r/5229/#review4269
::: toolkit/components/reader/AboutReader.jsm
(Diff revision 1)
> + this._win.setTimeout(() => this._setToolbarVisibility(true), 500);
Boo hacks. What causes the jankiness...is it a conflict with the dynamic URL bar? Might be worth mentioning here if so.
Attachment #8576400 -
Flags: review?(bnicholson)
Updated•10 years ago
|
Attachment #8576400 -
Flags: review+
Comment 13•10 years ago
|
||
Comment on attachment 8576400 [details]
MozReview Request: bz://998031/margaret
https://reviewboard.mozilla.org/r/5229/#review4271
Ship It!
Assignee | ||
Comment 14•10 years ago
|
||
(In reply to Brian Nicholson (:bnicholson) from comment #12)
> ::: toolkit/components/reader/AboutReader.jsm
> (Diff revision 1)
> > + this._win.setTimeout(() => this._setToolbarVisibility(true), 500);
>
> Boo hacks. What causes the jankiness...is it a conflict with the dynamic URL
> bar? Might be worth mentioning here if so.
I added a reference to bug 975533 in the comment. I believe that is the root issue (this is an issue that currently exists).
https://hg.mozilla.org/integration/fx-team/rev/6f7802ade852
Assignee | ||
Updated•10 years ago
|
status-firefox38:
--- → affected
Comment 16•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
Comment 17•10 years ago
|
||
When scrolling down a page, the reader mode toolbar slides off the screen and reappears when scrolling back up.
Also when tapping the screen once, the toolbar slides off the screen and reappears when tapping once again.
Verified fixed on:
Device: Nexus 4 (Android 4.4)
Build: Firefox for Android 39.0a1 (2015-03-18)
Comment 18•10 years ago
|
||
Comment 19•10 years ago
|
||
Verified fixed on:
Device: Nexus 5 (Android 5.0)
Build: Firefox for Android 38.0a2 (2015-03-27)
When scrolling down a page, the reader mode toolbar slides off the screen and reappears when scrolling back up.
Wwhen tapping the screen once, the toolbar slides off the screen and reappears when tapping once again.
Assignee | ||
Comment 20•9 years ago
|
||
Attachment #8576400 -
Attachment is obsolete: true
Attachment #8618123 -
Flags: review+
Assignee | ||
Comment 21•9 years ago
|
||
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•