MD Settings: Prevent unexpected scrolling to section on popstates.

MD Settings: Prevent unexpected scrolling to section on popstates.

On subpage-backs or Browser back buttons, prevent the MainPageBehavior
from scrolling to the position of the last routes' section.

This makes section routes behave more like #anchorLinks.

BUG=640523
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/776eb73066021f12a815c669c73c4db433751d15
Cr-Commit-Position: refs/heads/master@{#416386}

[tommycli, 2016-09-01 22:37:32]

Description was changed from

==========
MD Settings: Prevent unexpected scrolling to section on popstates.

On subpage-backs or Browser back buttons, prevent the MainPageBehavior
from scrolling to the position of the last routes' section.

This makes section routes behave more like #anchorLinks.

BUG=640523
==========

to

==========
MD Settings: Prevent unexpected scrolling to section on popstates.

On subpage-backs or Browser back buttons, prevent the MainPageBehavior
from scrolling to the position of the last routes' section.

This makes section routes behave more like #anchorLinks.

BUG=640523
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[tommycli, 2016-09-01 22:37:36]

The CQ bit was checked by tommycli@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-01 22:38:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tommycli, 2016-09-01 22:38:27]

The CQ bit was checked by tommycli@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-01 22:38:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tommycli, 2016-09-01 22:39:16]

tommycli@chromium.org changed reviewers:
+ michaelpg@chromium.org

[tommycli, 2016-09-01 22:39:17]

michaelpg: PTAL, thanks!

[commit-bot: I haz the power, 2016-09-01 23:53:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-01 23:53:51]

Dry run: This issue passed the CQ dry run.

[michaelpg, 2016-09-02 19:37:42]

https://codereview.chromium.org/2297663008/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/settings_page/main_page_behavior.js
(right):

https://codereview.chromium.org/2297663008/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/settings_page/main_page_behavior.js:107:
promise.then(this.tryTransitionToSection_.bind(this, isPopstate));
tryTransitionToSection transitions to whatever the current route is, so I'm not
sure we should be passing isPopstate like this. isPopstate should be based on
whether the most recent route was a popstate, not whether the route that happens
to be the first to call tryTransitionToSection_ after an animation was a
popstate.

Something like settings.isCurrentRoutePopstate() would work, if that would make
sense to have.

[tommycli, 2016-09-02 20:53:43]

michaelpg: thanks!

https://codereview.chromium.org/2297663008/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/settings_page/main_page_behavior.js
(right):

https://codereview.chromium.org/2297663008/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/settings_page/main_page_behavior.js:107:
promise.then(this.tryTransitionToSection_.bind(this, isPopstate));
On 2016/09/02 19:37:42, michaelpg wrote:
> tryTransitionToSection transitions to whatever the current route is, so I'm
not
> sure we should be passing isPopstate like this. isPopstate should be based on
> whether the most recent route was a popstate, not whether the route that
happens
> to be the first to call tryTransitionToSection_ after an animation was a
> popstate.
> 
> Something like settings.isCurrentRoutePopstate() would work, if that would
make
> sense to have.

Done.

[michaelpg, 2016-09-02 21:20:48]

nice, lgtm but a test for the popstate bool would be nice

[tommycli, 2016-09-02 22:17:43]

hi, I added a test thanks!

[tommycli, 2016-09-02 22:17:47]

The CQ bit was checked by tommycli@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-02 22:18:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tommycli, 2016-09-02 23:27:13]

The CQ bit was unchecked by tommycli@chromium.org

[tommycli, 2016-09-02 23:27:13]

The CQ bit was checked by tommycli@chromium.org

[tommycli, 2016-09-02 23:27:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/2297663008/#ps40001
(title: "add test")

[commit-bot: I haz the power, 2016-09-02 23:27:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-02 23:34:23]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-09-02 23:35:58]

Description was changed from

==========
MD Settings: Prevent unexpected scrolling to section on popstates.

On subpage-backs or Browser back buttons, prevent the MainPageBehavior
from scrolling to the position of the last routes' section.

This makes section routes behave more like #anchorLinks.

BUG=640523
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Prevent unexpected scrolling to section on popstates.

On subpage-backs or Browser back buttons, prevent the MainPageBehavior
from scrolling to the position of the last routes' section.

This makes section routes behave more like #anchorLinks.

BUG=640523
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/776eb73066021f12a815c669c73c4db433751d15
Cr-Commit-Position: refs/heads/master@{#416386}
==========

[commit-bot: I haz the power, 2016-09-02 23:35:59]

Patchset 3 (id:??) landed as
https://crrev.com/776eb73066021f12a815c669c73c4db433751d15
Cr-Commit-Position: refs/heads/master@{#416386}