Skip to content

fix: Allow ToC to scroll independently of page content #7754

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

utkarsh125
Copy link

Description

Fixed the issue of the Table of Contents (ToC) being statically positioned. The ToC now scrolls to the top under the heading instead of being sticky. Adjustments were made to ensure it blends seamlessly with the article layout at
/en/download/package-manager/all

fixes: #7740

Files Changed

packages/ui-components/Containers/MetaBar/index.tsx

packages/ui-components/Containers/MetaBar/index.module.css

Validation

Verified that the ToC no longer remains sticky and is correctly positioned beneath the article heading.

Confirmed visual alignment for larger screens
No regressions observed in MetaBar behavior

Video

ToC.Fix-1747407308773.mp4

Related Issues

fixes: #7740

Check List

I have read the Contributing Guidelines and made commit messages that follow the guideline.

  • I have run pnpm format to ensure the code follows the style guide.

  • I have run pnpm test to check if all tests are passing.

  • I have run pnpm build to check if the website builds without errors.

  • I've covered newly added functionality with unit tests if necessary.

@utkarsh125 utkarsh125 requested a review from a team as a code owner May 16, 2025 18:37
Copy link

vercel bot commented May 16, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview May 18, 2025 9:19pm

Comment on lines 15 to 16
lg:top-16
lg:max-h-[calc(100vh-4rem)]
Copy link
Member

Choose a reason for hiding this comment

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

I think this is causing the MetaBar to be too small:
image

Notice how it doesn't go all the way to the bottom

Copy link
Author

Choose a reason for hiding this comment

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

On it

Copy link
Author

Choose a reason for hiding this comment

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

Adding pb-12 kinda fixed the issue. The issue no longer persists, if there are any problems please let me know.

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Member

@avivkeller avivkeller May 17, 2025

Choose a reason for hiding this comment

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

I don't think it did. Is changing the max height even needed at all?

Copy link
Author

Choose a reason for hiding this comment

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

@avivkeller
Video Preview
It is need to give the sticky scroll to the wrapper, without it the ToC section scrolls in parallel to the main section
I have also fixed the issue where the metabar was too small.

Copy link
Contributor

github-actions bot commented May 18, 2025

Lighthouse Results

URL Performance Accessibility Best Practices SEO Report
/en 🟢 100 🟢 100 🟢 100 🟢 91 🔗
/en/about 🟢 100 🟢 100 🟢 100 🟠 82 🔗
/en/about/previous-releases 🟢 100 🟢 100 🟢 100 🟠 83 🔗
/en/download 🟠 88 🟢 100 🟢 100 🟢 91 🔗
/en/blog 🟢 100 🟢 100 🟢 96 🟢 92 🔗

@codecov-commenter
Copy link

codecov-commenter commented May 18, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.08%. Comparing base (fd87584) to head (22bd816).
Report is 2 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7754      +/-   ##
==========================================
+ Coverage   74.85%   75.08%   +0.22%     
==========================================
  Files          98       98              
  Lines        7895     7914      +19     
  Branches      200      196       -4     
==========================================
+ Hits         5910     5942      +32     
+ Misses       1984     1971      -13     
  Partials        1        1              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@avivkeller
Copy link
Member

Can you please check the visual regression: https://www.chromatic.com/test?appId=64c7d71358830e9105808652&id=6829e0dcf23a597de1aee02b

@utkarsh125
Copy link
Author

Can you please check the visual regression: https://www.chromatic.com/test?appId=64c7d71358830e9105808652&id=6829e0dcf23a597de1aee02b

@avivkeller I did make some changes, although I do not know how to test it. Can you let me know how can I test the chromatic diffs?

@avivkeller
Copy link
Member

avivkeller commented May 18, 2025

Check your storybook against the most recent main build. I can also run a diff.

@avivkeller
Copy link
Member

The issue is probably because of the added padding

@utkarsh125
Copy link
Author

The issue is probably because of the added padding

@avivkeller

I have removed the additional p-4 from
maybe this will solve the issue, I had added it to give more space to the Metabar from the bottom, I hope this can fix it.

also, I was talking about this specific link that you sent, how can I obtain it for the latest build?
https://www.chromatic.com/test?appId=64c7d71358830e9105808652&id=6829e0dcf23a597de1aee02b

This thing was pretty helpful because of the comparative visuals.

&:last-child {
      @apply mb-0
          p-4 <-------
    }

@utkarsh125
Copy link
Author

I think the chromatic diff is showing up because of sticky scroll bar, and in order to make sure the MetaBar isn't too small, I had changed the max-height to give some scrolling space to the wrapper in the MetaBar

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 this pull request may close these issues.

Allow ToC to scroll independently of page content
3 participants