61dae63 fix(shared-sidebar): tighten vertical spacing to match book/kits/labs/mlsysim
📊 Changes
1 file changed (+20 additions, -14 deletions)
View changed files
📝shared/styles/partials/_sidebar.scss (+20 -14)
📄 Description
Summary
Unifies sidebar vertical rhythm across all Quarto sites. Before this PR, the shared partial was loose (padding `4px 8px`, margin `1px 0`, explicit line-height `1.45`, extra padding and `margin-top` on section headers), while book/kits/labs/mlsysim already shipped their own tight local override (padding `2px 6px`, margin `0.5px 0`). Net effect: TinyTorch and the landing site looked noticeably airier than the book.
The comment was also wrong
The partial's own file header rationalized "more air than CSS-reset minimums to improve scanning" as a deliberate choice. That was horizontal-vs-vertical-spacing confusion:
Horizontal 6–8px padding DOES help mouse/touch targeting → keep
Vertical 4px padding + explicit margin-top on section headers makes a 60-item sidebar require more scrolling → remove
The book has the most items per sidebar in the ecosystem and it runs the tight form. If it's right there, it's right everywhere.
What changes
Selector
Before (loose)
After (tight)
`.sidebar-item a` padding
`4px 8px`
`2px 6px`
`.sidebar-item a` margin
`1px 0`
`0.5px 0`
`.sidebar-item a` line-height
`1.45` (explicit)
removed (Bootstrap default ~1.5)
Section header padding
`6px 8px`
removed (inherits from item rule)
Section header margin-top
`4px`
removed (no extra gap above sections)
Sites affected
TinyTorch, landing site: now tight (they import the shared partial directly) ✓
book, kits, labs, mlsysim: no visual change (their local overrides already specify these same tight values and take precedence)
slides, instructors: no sidebar so N/A
Follow-up (not in this PR)
Four sites still carry duplicate local copies of these rules. Each passes `${site}-accent` explicitly instead of the shared partial's `$accent` variable, which is why the duplicates exist. A follow-up could either:
Confirm each site defines `$accent` before the partial and delete the four local overrides → true single source of truth.
Or keep the per-site overrides but shrink them to "just the accent color" and inherit spacing from the partial.
Either option is a structural cleanup, not a visual change. Deferring until we have eyes on the deployed preview of this PR.
Test plan
`_sidebar.scss` scss syntax still parses (no local SCSS build, relies on downstream Quarto builds)
Visual audit in repo: confirmed TinyTorch imports this partial, confirmed book/kits/labs/mlsysim all ship local overrides that MATCH the new tight values
Post-merge: preview deploy on TinyTorch and eyeball the sidebar against book's sidebar side-by-side
🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.
## 📋 Pull Request Information
**Original PR:** https://github.com/harvard-edge/cs249r_book/pull/1514
**Author:** [@profvjreddi](https://github.com/profvjreddi)
**Created:** 4/24/2026
**Status:** ✅ Merged
**Merged:** 4/24/2026
**Merged by:** [@profvjreddi](https://github.com/profvjreddi)
**Base:** `dev` ← **Head:** `fix/shared-sidebar-tighten-spacing`
---
### 📝 Commits (1)
- [`61dae63`](https://github.com/harvard-edge/cs249r_book/commit/61dae6332d7a47ce89be44417c8fe7edde1f815c) fix(shared-sidebar): tighten vertical spacing to match book/kits/labs/mlsysim
### 📊 Changes
**1 file changed** (+20 additions, -14 deletions)
<details>
<summary>View changed files</summary>
📝 `shared/styles/partials/_sidebar.scss` (+20 -14)
</details>
### 📄 Description
## Summary
Unifies sidebar vertical rhythm across all Quarto sites. Before this PR, the shared partial was loose (padding \`4px 8px\`, margin \`1px 0\`, explicit line-height \`1.45\`, extra padding and \`margin-top\` on section headers), while book/kits/labs/mlsysim already shipped their own tight local override (padding \`2px 6px\`, margin \`0.5px 0\`). Net effect: TinyTorch and the landing site looked noticeably airier than the book.
## The comment was also wrong
The partial's own file header rationalized "more air than CSS-reset minimums to improve scanning" as a deliberate choice. That was horizontal-vs-vertical-spacing confusion:
- Horizontal 6–8px padding DOES help mouse/touch targeting → keep
- Vertical 4px padding + explicit margin-top on section headers makes a 60-item sidebar require more scrolling → remove
The book has the most items per sidebar in the ecosystem and it runs the tight form. If it's right there, it's right everywhere.
## What changes
| Selector | Before (loose) | After (tight) |
|---|---|---|
| \`.sidebar-item a\` padding | \`4px 8px\` | \`2px 6px\` |
| \`.sidebar-item a\` margin | \`1px 0\` | \`0.5px 0\` |
| \`.sidebar-item a\` line-height | \`1.45\` (explicit) | removed (Bootstrap default ~1.5) |
| Section header padding | \`6px 8px\` | removed (inherits from item rule) |
| Section header margin-top | \`4px\` | removed (no extra gap above sections) |
## Sites affected
- **TinyTorch, landing site**: now tight (they import the shared partial directly) ✓
- **book, kits, labs, mlsysim**: no visual change (their local overrides already specify these same tight values and take precedence)
- **slides, instructors**: no sidebar so N/A
## Follow-up (not in this PR)
Four sites still carry duplicate local copies of these rules. Each passes \`\$\{site\}-accent\` explicitly instead of the shared partial's \`\$accent\` variable, which is why the duplicates exist. A follow-up could either:
1. Confirm each site defines \`\$accent\` before the partial and delete the four local overrides → true single source of truth.
2. Or keep the per-site overrides but shrink them to "just the accent color" and inherit spacing from the partial.
Either option is a structural cleanup, not a visual change. Deferring until we have eyes on the deployed preview of this PR.
## Test plan
- [x] \`_sidebar.scss\` scss syntax still parses (no local SCSS build, relies on downstream Quarto builds)
- [x] Visual audit in repo: confirmed TinyTorch imports this partial, confirmed book/kits/labs/mlsysim all ship local overrides that MATCH the new tight values
- [ ] Post-merge: preview deploy on TinyTorch and eyeball the sidebar against book's sidebar side-by-side
---
<sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
📋 Pull Request Information
Original PR: https://github.com/harvard-edge/cs249r_book/pull/1514
Author: @profvjreddi
Created: 4/24/2026
Status: ✅ Merged
Merged: 4/24/2026
Merged by: @profvjreddi
Base:
dev← Head:fix/shared-sidebar-tighten-spacing📝 Commits (1)
61dae63fix(shared-sidebar): tighten vertical spacing to match book/kits/labs/mlsysim📊 Changes
1 file changed (+20 additions, -14 deletions)
View changed files
📝
shared/styles/partials/_sidebar.scss(+20 -14)📄 Description
Summary
Unifies sidebar vertical rhythm across all Quarto sites. Before this PR, the shared partial was loose (padding `4px 8px`, margin `1px 0`, explicit line-height `1.45`, extra padding and `margin-top` on section headers), while book/kits/labs/mlsysim already shipped their own tight local override (padding `2px 6px`, margin `0.5px 0`). Net effect: TinyTorch and the landing site looked noticeably airier than the book.
The comment was also wrong
The partial's own file header rationalized "more air than CSS-reset minimums to improve scanning" as a deliberate choice. That was horizontal-vs-vertical-spacing confusion:
The book has the most items per sidebar in the ecosystem and it runs the tight form. If it's right there, it's right everywhere.
What changes
Sites affected
Follow-up (not in this PR)
Four sites still carry duplicate local copies of these rules. Each passes `${site}-accent` explicitly instead of the shared partial's `$accent` variable, which is why the duplicates exist. A follow-up could either:
Either option is a structural cleanup, not a visual change. Deferring until we have eyes on the deployed preview of this PR.
Test plan
🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.