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

Addon-Docs: Change URL hash when TOC item is clicked and when scrolling #23618

Closed
wants to merge 2 commits into from

Conversation

sookmax
Copy link
Contributor

@sookmax sookmax commented Jul 26, 2023

What I did

Hello, I've noticed nothing actually happens to the url when the item of the table of contents for a docs page (if enabled) is clicked.

I thought it would be better for user experience if the current url actually changes to include the corresponding hash when a toc item is clicked. This PR includes that behavior.

While I was on it, I also added a behavior where the url hash changes reactively to the current scroll position and the current active link.

I've recorded short videos demonstrating the behaviors described above.

Url hash change on click

hash-change-onclick.mov

Url hash change on scroll

hash-change-onscroll-trimmed.mov

How to test

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli/src/sandbox-templates.ts
  • Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "build", "documentation", "maintenance", "dependencies", "other"]

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

@ndelangen
Copy link
Member

This is great @sookmax ! I've noticed some really amazing PRs from you ❤️

@ndelangen ndelangen changed the title Change URL hash when TOC item is clicked and when scrolling Addon-Docs: Change URL hash when TOC item is clicked and when scrolling Jul 26, 2023
@ndelangen ndelangen assigned JReinhold and unassigned ndelangen Nov 24, 2023
Copy link
Contributor

@JReinhold JReinhold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the behavior of changing the hash in the URL when a header-link is clicked, this is a pattern we see a lot of places.

I'm not a big fan of changing the hash automatically on scroll. I've never seen this UX pattern before and as a user I'd be surprised if this happened.

Thoughts @cdedreuille ?

@Sidnioulz
Copy link
Member

I like the behavior of changing the hash in the URL when a header-link is clicked, this is a pattern we see a lot of places.

I'm not a big fan of changing the hash automatically on scroll. I've never seen this UX pattern before and as a user I'd be surprised if this happened.

Thoughts @cdedreuille ?

@JReinhold That's something I see happen every now and then. I personally like it: the moment where I share a doc page to a third-party is likely the moment where I've encountered the piece of info I've been looking for. If the URL already links to the relevant section, it's likely faster for my third-party to locate the information in the document.

There's not much relevant contnent on the UX Stack exchange: https://ux.stackexchange.com/questions/30361/how-to-handle-history-events-from-scolling-and-other-in-page-events/30663#30663. And not all the answers on it are on-topic, sadly.

A quick look at major documentation tools shows they do not auto-inject the hash on scroll, so adopting the majority behaviour in the absence of hard UR data would mean not changing the hash.

@sookmax I'm happy to finalise the request changes for you if you no longer have time for it.

@sookmax
Copy link
Contributor Author

sookmax commented Dec 23, 2024

@sookmax I'm happy to finalise the request changes for you if you no longer have time for it.

Thanks @Sidnioulz. Feel free to pick up where I left off.

On automatic hash change on scroll: I'm pretty sure Vitepress had had this behavior when I created this doc: example. But I don't think that's the default case anymore, judging from their official docs' scrolling behavior.

@Sidnioulz
Copy link
Member

On automatic hash change on scroll: I'm pretty sure Vitepress had had this behavior when I created this doc: example. But I don't think that's the default case anymore, judging from their official docs' scrolling behavior.

Yeah I remember it that way too. It surprised me to see it had changed.

As I can't push directly to your branch, I've forked it and pushed it to #30130.

@Sidnioulz
Copy link
Member

@JReinhold @sookmax I'm taking the liberty to close this one now that the other one, rebased and with the requested changes and extra bug fixes, is open: #30130

@Sidnioulz Sidnioulz closed this Dec 24, 2024
@sookmax
Copy link
Contributor Author

sookmax commented Dec 24, 2024

Thanks for picking this up and finishing it @Sidnioulz, it is very much appreciated.

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

Successfully merging this pull request may close these issues.

5 participants