Closed Bug 998031 Opened 6 years ago Closed 5 years ago

Reader Mode toolbar should scroll in and out instead of fading

Categories

(Firefox for Android :: Reader View, defect)

30 Branch
All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 39
Tracking Status
firefox38 --- verified
firefox39 --- verified

People

(Reporter: ibarlow, Assigned: Margaret)

References

(Blocks 1 open bug)

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.
Whiteboard: [mentor=lucasr][lang=js]
Mentor: lucasr.at.mozilla
Whiteboard: [mentor=lucasr][lang=js] → [lang=js]
Margaret, would you like to mentor?
Flags: needinfo?(margaret.leibovic)
(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)
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)
Partial dupe of Bug 1114708.
Blocks: 1103232
No longer blocks: readerv2
OS: Mac OS X → Android
Hardware: x86 → All
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)
(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)
Blocks: readerv2
No longer blocks: 1103232
Blocks: 1120004
Assignee: nobody → margaret.leibovic
Mentor: margaret.leibovic
Whiteboard: [lang=js]
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)
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 :/
(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)
/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)
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 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)
Comment on attachment 8576400 [details]
MozReview Request: bz://998031/margaret

https://reviewboard.mozilla.org/r/5229/#review4271

Ship It!
(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
Duplicate of this bug: 1137829
https://hg.mozilla.org/mozilla-central/rev/6f7802ade852
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
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)
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.
Depends on: 1155083
Attachment #8576400 - Attachment is obsolete: true
Attachment #8618123 - Flags: review+
You need to log in before you can comment on or make changes to this bug.