nodejs.org icon indicating copy to clipboard operation
nodejs.org copied to clipboard

refactor/fix: Move sidebar from article to base layout

Open abizek opened this issue 1 year ago • 6 comments

Description

  • Moves the sidebar into the base layout to prevent rerender on page change
  • Moves the banner and navbar into the base layout and makes the navbar sticky
  • Improves responsiveness in pages using the article layout

Validation

Before After
Banner before-banner after-banner
Sidebar before-sidebar after-sidebar
Responsiveness

Related Issues

Fixes #6409 Possible fix for #6292

Check List

  • [x] I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • [x] I have run npx turbo format to ensure the code follows the style guide.
  • [x] I have run npx turbo test to check if all tests are passing.
  • [x] I have run npx turbo build to check if the website builds without errors.
  • [x] I've covered new added functionality with unit tests if necessary.

Please let me know if there is a better way to hide sidebars in not-found and error pages

abizek avatar Apr 08 '24 08:04 abizek

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

Name Status Preview Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview Apr 16, 2024 6:12am

vercel[bot] avatar Apr 08 '24 08:04 vercel[bot]

Possible fix for https://github.com/nodejs/nodejs.org/issues/6292

yes, it allows the banner to be moved, but the idea of the issue is to have a small cross that hides the banner

AugustinMauroy avatar Apr 08 '24 12:04 AugustinMauroy

Lighthouse Results

URL Performance Accessibility Best Practices SEO Report
/en 🔴 61 🟢 100 🟢 100 🟢 91 🔗
/en/about 🟢 98 🟢 100 🟢 100 🟢 91 🔗
/en/about/previous-releases 🟢 98 🟢 98 🟢 100 🟢 92 🔗
/en/download 🟢 98 🟢 100 🟢 100 🟢 91 🔗
/en/blog 🟢 99 🟢 100 🟢 96 🟢 92 🔗

github-actions[bot] avatar Apr 08 '24 12:04 github-actions[bot]

Unit Test Coverage Report

Lines Statements Branches Functions
Coverage: 92%
90.31% (597/661) 76.1% (172/226) 90.9% (120/132)

Unit Test Report

Tests Skipped Failures Errors Time
130 0 :zzz: 0 :x: 0 :fire: 5.594s :stopwatch:

github-actions[bot] avatar Apr 08 '24 12:04 github-actions[bot]

@ovflowd Cool. Take care and get well soon :heart:

abizek avatar Apr 09 '24 06:04 abizek

I will give scrolling to the previous position a try :+1:

abizek avatar Apr 15 '24 11:04 abizek

I'm closing this PR as I've found an extremely simple solution based on https://github.com/vercel/next.js/issues/49279

ovflowd avatar Apr 26 '24 11:04 ovflowd

I will give scrolling to the previous position a try 👍

I feel that in the end all these approaches are gimmicks. It definitely seems like a bug on Next.js side of things 😢

ovflowd avatar Apr 26 '24 11:04 ovflowd