Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

links on angular.io open new pages with scroll down the page #29893

Open
Frazer opened this Issue Apr 14, 2019 · 7 comments

Comments

4 participants
@Frazer
Copy link

Frazer commented Apr 14, 2019

馃摎 Docs or angular.io bug report

Description

When I scroll down a page on https://angular.io/, then click a link to another angular.io page, I would expect the new page to open at the top of that page. However the angular.io app keeps the current page scroll, so the new page opens scrolled down as far as the previous page.

馃敩 Minimal Reproduction

What's the affected URL?**

https://angular.io/

Reproduction Steps**

go to https://angular.io/, scroll down a page, click on a link.

Expected vs Actual Behavior**

expected: new page opens at the top of that page

actual: new page opens scrolled down the same distance as the last page

馃摲Screenshot

馃敟 Exception or Error





馃實 Your Environment

Browser info

chrome 73.0.3 on mac mojave

Anything else relevant?

@mohammedzamakhan

This comment has been minimized.

Copy link

mohammedzamakhan commented Apr 14, 2019

I do not see that issue when I access angular.io

@gkalpak

This comment has been minimized.

Copy link
Member

gkalpak commented Apr 15, 2019

I can't reproduce it either. We are supposed to explicitly scroll to top, on new navigations (i.e. no reloading or back/forward in history).

We did some changes recently to restore scroll positions, when reloading or navigating back/forward in history. It shouldn't mess with scroll position on other navigations (e.g. clicking on a link), but it is possible we have missed something (the code is quite complex).

@Frazer, I am going to close this, since it is not actionable without a way for us to reproduce it, but I would be happy to reopen and investigate, if we have a reproduction. So, thx for reporting and feel free to continue the discussion below.

Maybe you can provide more info to help us recognize patterns, possible culprits or ways to reproduce it. E.g.:

  • Are you seeing this on specific types of URLs (e.g. API docs, non-API docs, guides, other pages)?
  • What browsers does this happen (or not happen) on (other than Chrome 73 that you mentioned already)?
  • What devices does it happen on?
  • Does it happen consistently for you or only soemtimes?
  • Does it seem to be affected by other factors (network conditions, size of the page, etc.)?

cc @wKoza (who has written mostly of the scroll restoration logic)

@gkalpak gkalpak closed this Apr 15, 2019

@wKoza

This comment has been minimized.

Copy link
Contributor

wKoza commented Apr 15, 2019

Hi @gkalpak ,
After reflection, there is maybe a scenario which can be problematical. I've recognised that recently, we have a refresh when we click on a link (seems linked to the SW). In this case, I'm wondering if our script don't think that we are in this case of a reload. I'll check that.

@wKoza

This comment has been minimized.

Copy link
Contributor

wKoza commented Apr 16, 2019

@gkalpak , Can you explain me why, sometime, I have a refresh of angular.io when I click on a link. is it linked to the SW ?
In all cases, when it's happen, we don't remove the position from the session storage. To prevent this, we may call this method when we click on a link here .

@gkalpak

This comment has been minimized.

Copy link
Member

gkalpak commented Apr 16, 2019

Ah, good point, @wKoza. There indeed this case, where we do a "full" navigation when there is a SW update:

if (/^http/.test(url) || this.swUpdateActivated) {

For context, here is how this works:

  • The SW will periodically check for updates.
  • As soon as an update is found, the update is "activated" (which means that new clients will be served the new, updated assets (but existing clients will still see the old resources)).
  • As soon as the update is activated, the SW notifies existing clients. When this happens, the aio app will set a flag that the SW has an update (but not immediatelly relod the page to avoid interrupting the user).
  • The next time the user clicks on an in-app link, the app checks to see if the SW update flag is set and, if so, does a full navigation (which will result in being recognized as a new client and thus get the latest version from the SW).

So, it is very possible that we don't handle scroll restoration correctly in that case 馃榿

@wKoza

This comment has been minimized.

Copy link
Contributor

wKoza commented Apr 16, 2019

It was exactly what I searched ! A good old window.location. Maybe can you re-open this issue because removeStoredScrollPosition() is not trigged in this case.

@gkalpak gkalpak reopened this Apr 16, 2019

@gkalpak

This comment has been minimized.

Copy link
Member

gkalpak commented Apr 16, 2019

Thx for getting to the bottom of it, @wKoza. Great sleuthing 馃暤锔

@ngbot ngbot bot added this to the needsTriage milestone Apr 16, 2019

@gkalpak gkalpak added this to SELECTED FOR DEVELOPMENT in docs-infra Apr 16, 2019

wKoza added a commit to wKoza/angular that referenced this issue Apr 17, 2019

wKoza added a commit to wKoza/angular that referenced this issue Apr 17, 2019

wKoza added a commit to wKoza/angular that referenced this issue Apr 17, 2019

@wKoza wKoza referenced a pull request that will close this issue Apr 17, 2019

Open

fix(aio): remove scroll position from sessionStorage when a ServiceWo鈥 #29958

5 of 14 tasks complete

wKoza added a commit to wKoza/angular that referenced this issue Apr 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.