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

Anchors to questions in FAQ don't include header #212

Closed
ccordoba12 opened this issue Dec 14, 2020 · 5 comments
Closed

Anchors to questions in FAQ don't include header #212

ccordoba12 opened this issue Dec 14, 2020 · 5 comments
Assignees

Comments

@ccordoba12
Copy link
Member

This how things look after opening a certain FAQ anchor:

Selección_062

However, I think that looks odd and I'd really prefer that the header would be shown too, like this:

Selección_063

@CAM-Gerlach, could you take care of this? Thanks!

@CAM-Gerlach
Copy link
Member

CAM-Gerlach commented Dec 14, 2020

@ccordoba12 Could you please describe the browser, OS and exact steps to reproduce? Most of the code, complexity and commits in PR #207 is dedicated to avoiding this very issue, and as part of that PR, I successfully tested this specific behavior extensively across FF and Chrome on both Windows and Android in a variety of situations (and asked you and @juanis2112 to do the same on your platforms as part of your review).

The only scenario I had found (which my restesting now confirms) that behaved in an undesired fashion as in your above screenshot, which I believe I mentioned on both the PR and in the meeting, was if a user clicked the direct link button on an FAQ question again immediately after already clicking it once, or after the page had already been opened to the same question without clicking any other button in between. This is because the browser's built-in anchor scroll functionality is triggered, but it doesn't actually fire an anchor changed event (which is what triggers our custom JS) because the id anchor doesn't change.

I believe it should be possible to avoid this by adding our own custom event handler to the direct link buttons that fires whenever they are clicked regardless of whether the anchor changes and hacks around the core browser behavior, but as there isn't a clear reason why an average user (as opposed to one of us) would intentionally click the link button again on the the same question that's already open and scrolled to, I'm not sure its worth the time, complexity and bug potential that additional potentially tricky overriding of built-in behavior (that varies between browsers and platforms, and already required a fair amount of finagling to make it work otherwise) would require. However, if it can be reproduced in another realistic scenario, or on another platform/browser that none of us tested when reviewing the PR, then I can certainly take a look, so please let me know.

@ccordoba12
Copy link
Member Author

@ccordoba12 Could you please describe the browser, OS and exact steps to reproduce?

Firefox 78 on Linux.

The only scenario I had found (which my restesting now confirms) that behaved in an undesired fashion as in your above screenshot, which I believe I mentioned on both the PR and in the meeting, was if a user clicked the direct link button on an FAQ question again immediately after already clicking it once, or after the page had already been opened to the same question without clicking any other button in between.

In my case this happens every time I click on an anchor.

@CAM-Gerlach
Copy link
Member

CAM-Gerlach commented Dec 14, 2020

Okay, thanks. I cannot reproduce this behavior on FF 80 and Chrome 87 on Windows 8.1, nor on Falkon 3.0.0, QtWebEngine 5.10.1 or FF 83 on my own Linux VM (Fedora 29 LXQt). However, I can reproduce it some of the time on FF 78 on my Linux VM when clicking on the buttons or loading direct links, seemingly at random, which suggests to me its likely a race condition between the built-in browser functionality that scrolls to anchors and my code (which I was aware of and had added mitigations for which worked on all the systems I tested, but evidently they didn't go far enough for everything, which is why I wanted to make sure it was tested across a variety of browsers and platforms before behind merged). I'll set up a docs dev environment on my VM and see what I can do to fix this.

@ccordoba12
Copy link
Member Author

I'll set up a docs dev environment on my VM and see what I can do to fix this.

Thanks, but if this caused by an old browser, I think we can simply close it (I just haven't had time to update my distro).

I thought this was a general (and really bad) problem, but if it's working in newer versions, I see no reason to invest more time on it.

@ccordoba12
Copy link
Member Author

Closing because I verified this is not a problem in Firefox 84 on Linux.

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

No branches or pull requests

2 participants