New issue
Advanced search Search tips

Issue 841094 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Sep 6
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Task



Sign in to add a comment

Scroll to bottom of a long page should bring toolbars back

Project Member Reported by justincohen@chromium.org, May 9 2018

Issue description

Per offline discussion with kkhorimoto:

On phone, visit cnn.com
1) Scroll down
 secondary toolbar should disappear, primary toolbar shrinks
2) Scroll all the way down.
 secondary toolbar and primary toolbar should expand back.

Thoughts?
 
This is Safari's behavior when scrolling down a long page.  It wouldn't be too difficult to implement technically, but I'd like some input from Mardini about whether this is something we'd like to do.
Yeah. I think it's a nice-to-have for sure but I wouldn't prioritize this unless all higher priority issues have been resolved. You can get back your expanded toolbars by just scrolling up a bit after you've reached the bottom, right? So it's not like a dead-end when you hit the bottom. 

Cc: mard...@chromium.org
Labels: MS-Fullscreen Proj-UIRefresh MS-Toolbar
Owner: ----
Status: Available (was: Assigned)

Would bringing the toolbar back at the bottom of the page fix issue 841297 ?
If so, it might be worth doing this one then. Thoughts?
Labels: -MS-Toolbar MS-AdaptiveToolbar
Labels: -MS-AdaptiveToolbar MS-Adaptive-Toolbar
#WhyNoAutoCompleteForLabels
This is different from Issue 841297.  That's an edge case that occurs when a page's rendered height is larger than the viewport size when toolbars are visible, but smaller than when toolbars are hidden.

This shouldn't be overly difficult to implement after some refactorings have landed.
Labels: -Pri-2 -MS-Adaptive-Toolbar -MS-Fullscreen M-70 Pri-1
Owner: kkhorimoto@chromium.org
Status: Assigned (was: Available)
This seems like a nice-to-have polish improvement for M70.

@kkhorimoto is this easier to implement now?
Yes, I agree that this should be added for M70.  Now that we've upgraded EG, the refactors can be landed without causing test failures.  I'll land those and add this behavior for M70.
Thank you, Kurt. Speaking of bringing toolbars back. Is there a bug for tracking bringing the bottom bar back up when the user has long-pressed on a link and chose open in new tab? Currently, with the animation we have in place, if we're in full screen the animation falls into the abyss :). Should we consider bringing the toolbars momentarily up to show where the tab is then dismiss it?
I am waiting for Pete to come back to start working on  issue 865863 .
Status: Started (was: Assigned)
Project Member

Comment 13 by bugdroid1@chromium.org, Sep 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f6f1f1913373057a25de1189295bd916bacd855f

commit f6f1f1913373057a25de1189295bd916bacd855f
Author: Kurt Horimoto <kkhorimoto@chromium.org>
Date: Thu Sep 06 23:12:03 2018

[iOS] Show toolbars after scrolling to the bottom of the page.

This CL updates the behavior of fullscreen such that the toolbars will
be re-shown when the user starts a scroll past the bottom of a page
when the toolbars are completely collapsed.

Bug:  841094 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I9e2cb3a59e931f83bb43e8a68df48a80bb6d0c3b
Reviewed-on: https://chromium-review.googlesource.com/1200563
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589357}
[modify] https://crrev.com/f6f1f1913373057a25de1189295bd916bacd855f/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/f6f1f1913373057a25de1189295bd916bacd855f/ios/chrome/browser/ui/fullscreen/fullscreen_mediator.mm
[modify] https://crrev.com/f6f1f1913373057a25de1189295bd916bacd855f/ios/chrome/browser/ui/fullscreen/fullscreen_model.h
[modify] https://crrev.com/f6f1f1913373057a25de1189295bd916bacd855f/ios/chrome/browser/ui/fullscreen/fullscreen_model.mm

Labels: Merge-Approved-70
Status: Fixed (was: Started)
Labels: -Merge-Approved-70 Merge-Request-70
Project Member

Comment 16 by sheriffbot@chromium.org, Sep 7

Labels: -Merge-Request-70 Hotlist-Merge-Approved Merge-Approved-70
Your change meets the bar and is auto-approved for M70. Please go ahead and merge the CL to branch 3538 manually. Please contact milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 17 by bugdroid1@chromium.org, Sep 10

Labels: -merge-approved-70 merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1ba827376d32509bf4dac76022e2eab5986227c8

commit 1ba827376d32509bf4dac76022e2eab5986227c8
Author: Kurt Horimoto <kkhorimoto@chromium.org>
Date: Mon Sep 10 18:22:42 2018

[iOS] Show toolbars after scrolling to the bottom of the page.

This CL updates the behavior of fullscreen such that the toolbars will
be re-shown when the user starts a scroll past the bottom of a page
when the toolbars are completely collapsed.

Bug:  841094 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I9e2cb3a59e931f83bb43e8a68df48a80bb6d0c3b
Reviewed-on: https://chromium-review.googlesource.com/1200563
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#589357}(cherry picked from commit f6f1f1913373057a25de1189295bd916bacd855f)
Reviewed-on: https://chromium-review.googlesource.com/1216810
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#233}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/1ba827376d32509bf4dac76022e2eab5986227c8/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/1ba827376d32509bf4dac76022e2eab5986227c8/ios/chrome/browser/ui/fullscreen/fullscreen_mediator.mm
[modify] https://crrev.com/1ba827376d32509bf4dac76022e2eab5986227c8/ios/chrome/browser/ui/fullscreen/fullscreen_model.h
[modify] https://crrev.com/1ba827376d32509bf4dac76022e2eab5986227c8/ios/chrome/browser/ui/fullscreen/fullscreen_model.mm

Status: Verified (was: Fixed)
Verified in 70.0.3538.17 beta in iPhone 7(iOS 11.4.1), iPhone 6s plus(iOS 12 beta 12), iPad Air(iOS 12 beta 12) and iPad Air(iOS 10.3.3)

The hidden toolbars are re-shown when the user starts scrolling past the boottom of the page, looks good.

Link to video: https://drive.google.com/file/d/1ZALS3sdB5NRW1bUBLpBhI0aJsnHd_3cL/view?usp=sharing

Issue 862866 has been merged into this issue.

Sign in to add a comment