feat(#1932): implement smooth scrolling for anchor navigation across site #1965

Mohit5Upadhyay · 2025-07-07T10:55:01Z

Description

This PR implements smooth scrolling for in-page navigation across the Express.js website as mentioned in Issue : #1932

This improves the user experience by ensuring that navigation to sections via anchor links transitions smoothly, rather than jumping abruptly.

Closes #1932

Video

smooth-scrolling.mp4

…n across site

netlify · 2025-07-07T10:55:05Z

✅ Deploy Preview for expressjscom-preview ready!

Name	Link
🔨 Latest commit	`9ce8ec4`
🔍 Latest deploy log	https://app.netlify.com/projects/expressjscom-preview/deploys/68760cd4d55d1c00083cf01e
😎 Deploy Preview	https://deploy-preview-1965--expressjscom-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

ShubhamOulkar · 2025-07-07T12:39:04Z

js/app.js

+  $(document).on('click', 'a[href^="#"]:not([href="#"]):not([href="#top"])', function(e) {
+    var href = $(this).attr('href');
+    if (smoothScrollToElement(href)) {
+      e.preventDefault();


Can you please remove this line https://github.com/expressjs/expressjs.com/blob/gh-pages/js/menu.js#L186 ? Since default behavior stopped here.

ShubhamOulkar · 2025-07-07T12:56:45Z

js/app.js

  // top link
  $('#top').click(function(e){
    $('html, body').animate({scrollTop : 0}, 500);
    return false;
  });

+  // Smooth scrolling for all anchor links
+  $(document).on('click', 'a[href^="#"]:not([href="#"]):not([href="#top"])', function(e) {


Suggested change

$(document).on('click', 'a[href^="#"]:not([href="#"]):not([href="#top"])', function(e) {

$("#api-doc").on('click', 'a[href^="#"]:not([href="#"]):not([href="#top"])', function(e) {

We should minimize the scope of this click event. Currently, $(document).on('click'...) attaches the handler to all anchor links with # across every page. However, smooth scrolling is only needed on API pages, since the TOC exists only there. Scoping it to #api-doc avoids unnecessary event handling and prevents inconsistent anchor # behavior on other pages. Also, checking "#api-doc" exist on page is required before applying click event.

bjohansebas

Thanks for the PR!

As i mentioned in the issue, this can be handled through CSS, javascript is not necessary.

ShubhamOulkar · 2025-07-08T03:20:27Z

this can be handled through CSS,

I am OK with CSS only solution. But this approach comes with some limitations, like we cannot control scroll behavior and can not add header offsets. But it is fine if headings are not hiding behind the header bar on anchor link click.

Mohit5Upadhyay · 2025-07-08T09:25:27Z

Thanks @bjohansebas for the suggestion about a CSS-only solution!
Unfortunately, I think for this project, CSS alone isn’t sufficient because:

IDs like #app.METHOD (http://localhost:4000/en/5x/api.html#app.listen) contain dots, which CSS can’t reliably scroll to without custom JS.
We need a precise scroll offset (120px) to avoid headings hiding behind the navbar.

So I’d propose keeping the JS smooth scroll, scoped only to #api-doc to avoid global impact as per suggested by @ShubhamOulkar.

Let me know if you’d prefer another approach!
Thanks again for your feedback.

bjohansebas · 2025-07-11T18:45:50Z

But it is fine if headings are not hiding behind the header bar on anchor link click.

The headings are never hidden by the header bar, or at least I’ve been trying to reproduce it and haven’t been able to.

IDs like #app.METHOD (http://localhost:4000/en/5x/api.html#app.listen) contain dots, which CSS can’t reliably scroll to without custom JS.

That doesn’t matter in CSS, since the effect will be applied to all elements anyway.

* {
  scroll-behavior: smooth;
}

Mohit5Upadhyay · 2025-07-12T16:02:56Z

Hi @bjohansebas and @ShubhamOulkar,

I’ve updated the implementation to follow your suggestion of a CSS-only smooth scrolling solution.

I removed all the custom JavaScript smooth scroll code I previously added.
Instead, I’ve applied this in CSS:

* {
  scroll-behavior: smooth;
}

h1, h2, h3, h4, h5, h6,
[id] {
  scroll-margin-top: 120px;
}

@media (prefers-reduced-motion: reduce) {
  html {
    scroll-behavior: auto;
  }
}

Let me know if you’d like any further changes!
Thanks for your time! 🙏

bjohansebas · 2025-07-15T02:32:05Z

css/style.css

+h1, h2, h3, h4, h5, h6,
+[id] {
+  scroll-margin-top: 120px;
+}


I'm not sure if this is actually doing anything, and if it's supposed to have the same effect as in the MDN example https://developer.mozilla.org/en-US/docs/Web/CSS/scroll-margin-top, then I'm -1 on this, I don't think we should do it.

without scroll top margin with scroll top margin

We can clearly see that margin spaces added to the header on scroll from the top of the viewport.
This property is used to increase visible area around the target element so that it is accessible to the user.

In our context, why it is important?
I expressed my concerns over the issue of hiding headers behind the top navigation bar in a comment above.

But it is fine if headings are not hiding behind the header bar on anchor link click.

This happens if top navigation is positioned sticky at the top. See image, express.raw() hide behind sticky top navigation.

It is not required right now, without scroll top margin, header scroll into viewport at top 0px. But I think adding some small margin 60px to the top looks good for me. Also our header is 57px height. Maybe I will go with sticky top navigation bar in #1943.

see scroll_padding_and_margin

ShubhamOulkar · 2025-07-15T06:00:32Z

css/style.css

+/* handle header offset with scroll-margin */
+h1, h2, h3, h4, h5, h6,
+[id] {
+  scroll-margin-top: 120px;


Suggested change

scroll-margin-top: 120px;

scroll-margin-top: 60px;

see comment https://github.com/expressjs/expressjs.com/pull/1965/files#r2206464995

ShubhamOulkar · 2025-07-15T06:10:14Z

css/style.css

@@ -264,11 +278,6 @@ a {
  padding-top: 40px;
 }

-#api-doc *:target, #page-doc *:target {


I support removing this
since we are using scroll top margin only on headers.

ShubhamOulkar · 2025-07-15T06:11:42Z

css/style.css

+}
+
+/* Respect user motion preferences for access */
+@media (prefers-reduced-motion: reduce) {


It is Good practice to consider accessibility
+1

ShubhamOulkar

Please do remove this line https://github.com/expressjs/expressjs.com/blob/gh-pages/js/menu.js#L186. On TOC main section heading click, it should scroll to main sections on the page. Currently, it is disabled. Only subsections get scrolled.

…reventDefault for TOC links

Mohit5Upadhyay · 2025-07-15T08:18:19Z

Hi @bjohansebas and @ShubhamOulkar,

Thanks so much for the thoughtful reviews!

Regarding scroll-margin-top:
I agree with @ShubhamOulkar that it’s helpful to prevent headings from hiding behind the sticky nav bar.

But I think adding some small margin 60px to the top looks good for me. Also our header is 57px height. Maybe I will go with sticky top navigation bar in #1943.

I’ll adjust the value to 60px as suggested, since our header height is ~57px.

Remove event.preventDefault() in menu.js so main TOC sections scroll as expected.

Let me know if there’s anything else you’d like tweaked!
Thanks again for your guidance.

feat(expressjs#1932): implement smooth scrolling for anchor navigatio…

ee25a7b

…n across site

Mohit5Upadhyay requested a review from a team as a code owner July 7, 2025 10:55

ShubhamOulkar requested changes Jul 7, 2025

View reviewed changes

bjohansebas requested changes Jul 7, 2025

View reviewed changes

feat: implement CSS-only smooth scrolling for anchor navigation

736831c

bjohansebas reviewed Jul 15, 2025

View reviewed changes

ShubhamOulkar reviewed Jul 15, 2025

View reviewed changes

ShubhamOulkar requested changes Jul 15, 2025

View reviewed changes

apply reviewer changes: Update scroll-margin-top to 60px and remove p…

9ce8ec4

…reventDefault for TOC links

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(#1932): implement smooth scrolling for anchor navigation across site #1965

feat(#1932): implement smooth scrolling for anchor navigation across site #1965

Uh oh!

Mohit5Upadhyay commented Jul 7, 2025

Uh oh!

netlify bot commented Jul 7, 2025 •

edited

Loading

Uh oh!

ShubhamOulkar Jul 7, 2025

Uh oh!

ShubhamOulkar Jul 7, 2025

Uh oh!

bjohansebas left a comment

Uh oh!

ShubhamOulkar commented Jul 8, 2025

Uh oh!

Mohit5Upadhyay commented Jul 8, 2025

Uh oh!

bjohansebas commented Jul 11, 2025

Uh oh!

Mohit5Upadhyay commented Jul 12, 2025

Uh oh!

bjohansebas Jul 15, 2025

Uh oh!

ShubhamOulkar Jul 15, 2025 •

edited

Loading

Uh oh!

ShubhamOulkar Jul 15, 2025

Uh oh!

ShubhamOulkar Jul 15, 2025

Uh oh!

ShubhamOulkar Jul 15, 2025

Uh oh!

ShubhamOulkar left a comment

Uh oh!

Mohit5Upadhyay commented Jul 15, 2025

Uh oh!

Uh oh!

	$(document).on('click', 'a[href^="#"]:not([href="#"]):not([href="#top"])', function(e) {
	$("#api-doc").on('click', 'a[href^="#"]:not([href="#"]):not([href="#top"])', function(e) {

feat(#1932): implement smooth scrolling for anchor navigation across site #1965

Are you sure you want to change the base?

feat(#1932): implement smooth scrolling for anchor navigation across site #1965

Uh oh!

Conversation

Mohit5Upadhyay commented Jul 7, 2025

Description

Video

Uh oh!

netlify bot commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for expressjscom-preview ready!

Uh oh!

ShubhamOulkar Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

ShubhamOulkar Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

bjohansebas left a comment

Choose a reason for hiding this comment

Uh oh!

ShubhamOulkar commented Jul 8, 2025

Uh oh!

Mohit5Upadhyay commented Jul 8, 2025

Uh oh!

bjohansebas commented Jul 11, 2025

Uh oh!

Mohit5Upadhyay commented Jul 12, 2025

Uh oh!

bjohansebas Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

ShubhamOulkar Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShubhamOulkar Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

ShubhamOulkar Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

ShubhamOulkar Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

ShubhamOulkar left a comment

Choose a reason for hiding this comment

Uh oh!

Mohit5Upadhyay commented Jul 15, 2025

Uh oh!

Uh oh!

netlify bot commented Jul 7, 2025 •

edited

Loading

ShubhamOulkar Jul 15, 2025 •

edited

Loading