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

Track hashchange event #136

Closed
moetelo opened this issue Apr 14, 2022 · 10 comments · Fixed by #137
Closed

Track hashchange event #136

moetelo opened this issue Apr 14, 2022 · 10 comments · Fixed by #137

Comments

@moetelo
Copy link
Contributor

moetelo commented Apr 14, 2022

In Chrome, if you set a new text fragment hash programmatically, like
https://github.com/GoogleChromeLabs/text-fragments-polyfill
location.hash = '#:~:text=module%20of%20util-,functions,-for%20generating%20URLs';
, the page will be scrolled to the text.

The behavior does not provide such functionality.

@moetelo
Copy link
Contributor Author

moetelo commented Apr 14, 2022

My guess is that the polyfill should match Chrome's behavior, so the PR is here: #137.

@ivan
Copy link

ivan commented Dec 1, 2022

I noticed this as well in Firefox by just changing the #:~:text= in the URL bar and pressing Enter. Chrome (tested 108) highlights and scrolls to the new text, while the polyfill does not.

@tomayac
Copy link
Member

tomayac commented Dec 1, 2022

@bokand, what do you think is the right behavior here?

@bokand
Copy link
Collaborator

bokand commented Dec 1, 2022

There's some context in WICG/scroll-to-text-fragment#74

Given the way STTF is spec'd and implemented, a hash change should cause a scroll-to-text since it invokes the "scroll to the fragment" algorithm which is where STTF is invoked.

@tomayac
Copy link
Member

tomayac commented Dec 2, 2022

Thanks, @bokand! So @tfmar, are you happy to merge #137?

@ZiadJ
Copy link

ZiadJ commented Jan 23, 2023

Any reason why this is not merged yet?

@tomayac
Copy link
Member

tomayac commented Jan 24, 2023

@bokand @tfmar is anyone from the team owning this?

@bokand
Copy link
Collaborator

bokand commented Jan 26, 2023

Apologies for the delay - looks good with a comment I left on the review.

@moetelo
Copy link
Contributor Author

moetelo commented Jan 26, 2023

@bokand Can't find your comment. Could you request changes directly on the PR, if there are any issues?

@bokand
Copy link
Collaborator

bokand commented Jan 26, 2023

Sorry - I forgot to hit "submit". You should see it now

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

Successfully merging a pull request may close this issue.

5 participants