Skip to content

doc: add a smooth scrolling effect to the sidebar #59007

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.

Sign up for GitHub

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

Jump to bottom
Merged
nodejs-github-bot merged 1 commit into from
Dec 24, 2025
Merged

doc: add a smooth scrolling effect to the sidebar #59007

nodejs-github-bot merged 1 commit into from
Dec 24, 2025

Conversation

@btea
Copy link
Contributor

@btea btea commented Jul 9, 2025

When searching for the API in the Quick Flower Cutting module, each time you switch to a different module, the content in the sidebar scrolls to the top, and the currently active link is not immediately visible.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 9, 2025

Review requested:

  • @nodejs/nodejs-website

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Jul 9, 2025
@btea btea changed the title docs: inject sidebar active link scroll into view script doc: inject sidebar active link scroll into view script Jul 9, 2025
avivkeller
avivkeller requested changes Jul 9, 2025
View reviewed changes
Copy link
Member

@avivkeller avivkeller left a comment

Choose a reason for hiding this comment

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

We shouldn't add JavaScript unless it's absolutely needed.

If it is, this should added to the assets, rather than directly to the HTML

avivkeller
avivkeller reviewed Jul 9, 2025
View reviewed changes
@btea btea force-pushed the docs/inject-scrollintoview-script branch from a257f95 to defccdf Compare July 9, 2025 14:53
@btea
Copy link
Contributor Author

btea commented Jul 9, 2025

We shouldn't add JavaScript unless it's absolutely needed.

If it is, this should added to the assets, rather than directly to the HTML

Thanks for the tip, it would be nice to add automatic scrolling of active sidebar links into the view area.

avivkeller
avivkeller requested changes Jul 9, 2025
View reviewed changes
Copy link
Member

@avivkeller avivkeller left a comment

Choose a reason for hiding this comment

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

Please lint, fix the casing, and add comments

@btea btea force-pushed the docs/inject-scrollintoview-script branch from 08d3e30 to 09348d4 Compare July 10, 2025 00:07
@btea
Copy link
Contributor Author

btea commented Jul 10, 2025

Done.

mikeesto
mikeesto reviewed Jul 11, 2025
View reviewed changes
Copy link
Member

@mikeesto mikeesto left a comment

Choose a reason for hiding this comment

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

FYI, CI is failing because the first commit starts with fix, it should be doc

@btea btea force-pushed the docs/inject-scrollintoview-script branch from 09348d4 to 6643d19 Compare July 12, 2025 04:05
@btea
Copy link
Contributor Author

btea commented Jul 12, 2025

I have modified the commit information.

@btea
Copy link
Contributor Author

btea commented Dec 22, 2025 •
edited
Loading

Turn it off because #60410 did the same thing.

Add a smooth scrolling effect to the sidebar.

@btea btea closed this Dec 22, 2025
@btea btea deleted the docs/inject-scrollintoview-script branch December 22, 2025 04:58
@btea btea restored the docs/inject-scrollintoview-script branch December 22, 2025 05:42
@btea btea reopened this Dec 22, 2025
@btea btea changed the title doc: inject sidebar active link scroll into view script doc: add a smooth scrolling effect to the sidebar. Dec 22, 2025
@btea btea changed the title doc: add a smooth scrolling effect to the sidebar. doc: add a smooth scrolling effect to the sidebar Dec 22, 2025
@btea btea mentioned this pull request Dec 23, 2025
Merged
4 tasks
@avivkeller avivkeller added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Dec 23, 2025
@avivkeller
Copy link
Member

avivkeller commented Dec 23, 2025

@btea can you amend the first commit to match the PR title?

@btea
Copy link
Contributor Author

btea commented Dec 23, 2025

Previously, due to a conflict with the main branch, I synced the main branch's code, and the latest commit history shows commits from the main branch. I don't know how to resolve this.

@avivkeller
Copy link
Member

avivkeller commented Dec 24, 2025

No worries! Make you sure toggle "Maintainers can edit", and I'll rebase it myself when I get a free moment.

@btea
Copy link
Contributor Author

btea commented Dec 24, 2025

Thank you, I've already checked the box.

@avivkeller avivkeller force-pushed the docs/inject-scrollintoview-script branch from d47ca88 to eafe0b1 Compare December 24, 2025 01:56
@avivkeller avivkeller added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Dec 24, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 24, 2025
@nodejs-github-bot nodejs-github-bot merged commit ae407c3 into nodejs:main Dec 24, 2025
19 checks passed
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 24, 2025

Landed in ae407c3

@btea btea deleted the docs/inject-scrollintoview-script branch December 24, 2025 03:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@avivkeller avivkeller avivkeller approved these changes

+2 more reviewers

@mikeesto mikeesto mikeesto left review comments

@AugustinMauroy AugustinMauroy AugustinMauroy approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@btea @nodejs-github-bot @avivkeller @mikeesto @AugustinMauroy