-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
x/pkgsite: jump to "x" close button broken #52502
Comments
We've seen this report before (#50895), I'm finding this hard to reproduce. Does it happen consistently for you, or are you able to provide detailed steps? It might be less work to reimplement these components than track this down. They were created before widespread browser support for the HTML dialog element and make use of dialog polyfill that is no longer necessary. |
Hmm, it seems to be hit-or-miss when navigating from another page, and also seems to work when I close a tab and reopen it (ctrl-shift-T), but refreshing the page seems to break it pretty reliably. In a tab where it works, a breakpoint at line 43 of modal.ts is hit: for (const btn of this.el.querySelectorAll<HTMLButtonElement>('[data-modal-close]')) {
btn.addEventListener('click', () => {
if (this.el.close) { // <----- breakpoint here is hit when clicking close
this.el.close();
} else {
this.el.removeAttribute('opened');
}
});
} But on a tab that dosen't work that breakpoint isn't hit. I also put a breakpoint in the constructor that registers that event handler. When refreshing the page, the constructor isn't called, but when doing a hard refresh (ctrl-shift-R) it is (and subsequently the button works). Seems like maybe a race condition, but I haven't been able to narrow it down any more. |
When the js file is cached in the browser, it's very likely that it will be executed before the DOM is ready. In this case, the returned elements of a query is not the expected ones most of the time. So the js code that accesses elements should be delayed until the page is fully loaded. Fixes golang/go#52502
Change https://go.dev/cl/401914 mentions this issue: |
I can reproduce the issue like this:
I have sent a PR to fix this issue by delaying the execution until the page is fully loaded. |
Thank you for looking into this! |
What is the URL of the page with the issue?
https://pkg.go.dev/bytes@go1.18.1, but I think all packages have this problem.
What is your user agent?
Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.127 Safari/537.36
Screenshot
What did you do?
Click the "Jump To" input in the left panel, click the
X
button in the top-right of the popupWhat did you expect to see?
The popup closes.
What did you see instead?
Nothing happens. No errors captured in the js console.
The close button (bottom right) works.
The text was updated successfully, but these errors were encountered: