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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scroll to anchor behavior is different when a anchor link is opened in a different tab #1150

Open
damithc opened this issue Mar 26, 2020 · 12 comments

Comments

@damithc
Copy link
Contributor

damithc commented Mar 26, 2020

v2.12.0, Chrome, Windows

Ctrl+Click (to open in new tab) on this link

Expected (this is what you get if you normal-click on the link):
image

Actual:
image

@damithc
Copy link
Contributor Author

damithc commented Mar 26, 2020

@le0tan can take a look?

@le0tan
Copy link
Contributor

le0tan commented Mar 26, 2020

@damithc I can't reproduce this bug. Is this bug still there after restarting the browser?

@damithc
Copy link
Contributor Author

damithc commented Mar 26, 2020

@le0tan Noticed this in two computers. Yes, can reproduce. May be specific to OS and Browser?

@le0tan
Copy link
Contributor

le0tan commented Mar 26, 2020

@damithc I'm on Windows as well, which version of Chrome are you using?

@damithc
Copy link
Contributor Author

damithc commented Mar 26, 2020

Tried on Edge. Sometimes work, sometimes end up in at the totally wrong place.
image

@le0tan
Copy link
Contributor

le0tan commented Mar 26, 2020

I now can reproduce this using Firefox. I can confirm that the anchors are correctly generated and scrollToUrlAnchorHeading is working correctly (i.e. it shouldn't be caused by the fixed navbar feature). Probably has something to do with the order of executing the scripts, investigating.

@openorclose
Copy link
Contributor

openorclose commented Mar 28, 2020

Could it be due to expanded panels?

There are quite a few expanded panels in the 2113 website.

I faced the same problem on the 2113 website on Ubuntu 18.04 Firefox.

But I tried on

https://markbind.org/userGuide/fullSyntaxReference.html#lists

and it always scrolled to the correct heading.

@le0tan
Copy link
Contributor

le0tan commented Mar 28, 2020

Could it be due to expanded panels?

There are quite a few expanded panels in the 2113 website.

I faced the same problem on the 2113 website on Ubuntu 18.04 Firefox.

But I tried on

https://markbind.org/userGuide/fullSyntaxReference.html#lists

and it always scrolled to the correct heading.

Not likely as I tried on the MarkBind doc, and there's no expanded panels in the documentation.

@tlylt
Copy link
Contributor

tlylt commented Jun 23, 2022

Just encountered this. My finding is that if the anchor link is

  • left-click and view on the same page -> the scroll works as expected
  • right-click and open on a new tab, and immediately click into that tab before it has finished rendering -> the scroll works as expected (not sure if this is the always true but looks like it)

The error occurs when you

  • right-click and open on a new tab, wait for a few seconds and click into that tab -> the scroll covers the heading as shown in the original post.

Reproduction step:

Recording:
5EtHcUVINj

Not sure but it might be related to #1948

@ang-zeyu
Copy link
Contributor

could it simply be due to the ordering? (should be swapped) (still, odd that it works using a "normal" click to open)

  scrollToUrlAnchorHeading();
  detectAndApplyHeaderStyles();

I think we could try adapting this part to use the detected sticky header height as well, if any

  window.scrollBy(0, -document.body.style.paddingTop.replace('px', ''));

@EltonGohJH
Copy link
Contributor

EltonGohJH commented Mar 26, 2023

I did some research on this.
This issue should only be attempted after #1953 is merged.

The issue with this is that scrollIntoView does not work when the document is not visible on our end.

Some solutions that I have attempted but to no avail.

  1. One time event listen to visibility change. On visbility change to visible, run scrollIntoView.
    1a. This work but somehow scroll margin top is not of the correct value on switch to new tab. So, the nav bar will cover the header.
  2. Also attempted to scroll to position with offset.
    2a. Same problem as 1a.

Somehow, even if we perform scrolling on visible and after onLoad, the scroll margin top value is still incorrect initially.

I only have hacky ideas to fix it.

  1. We can follow detectAndApplyHeaderStyles in the same file as scrollToUrlAnchorHeading to retrieve --sticky-header-height which can be used to scroll it correctly with scroll to position with offset.
    1a. Potential issue would be that what if we have some element that has a scroll-margin-top that is not equal to --sticky-header-height.
  2. Through observer or some event loop, to check for scroll margin top and only scroll when the value is updated.

@ang-zeyu
Copy link
Contributor

In addition,

  • Scroll-to-anchor is broken on mobile (physical android) chrome when opening in new tab irregardless of how fast I switch to the new tab (or maybe I'm just not quick enough.. 🤷‍♂️).

    retrieve --sticky-header-height which can be used to scroll it correctly with scroll to position with offset.

    The offset solution also does not work in this case.

  • Consistently fine on Safari (Mac) but maybe I haven't tried enough times.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants