refactor/fix: Move sidebar from article to base layout
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 | ||
| 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 formatto ensure the code follows the style guide. - [x] I have run
npx turbo testto check if all tests are passing. - [x] I have run
npx turbo buildto 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
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 |
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
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 | 🔗 |
Unit Test Coverage Report
| Lines | Statements | Branches | Functions |
|---|---|---|---|
| 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: |
@ovflowd Cool. Take care and get well soon :heart:
I will give scrolling to the previous position a try :+1:
I'm closing this PR as I've found an extremely simple solution based on https://github.com/vercel/next.js/issues/49279
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 😢