[PR #1515] [MERGED] refactor(sidebar): consolidate to single source of truth (-1508 lines) #9181

New Issue

GiteaMirror · 2026-05-03T01:25:14-05:00

GiteaMirror commented

2026-05-03 01:25:14 -05:00

📋 Pull Request Information

Original PR: https://github.com/harvard-edge/cs249r_book/pull/1515
Author: @profvjreddi
Created: 4/24/2026
Status: ✅ Merged
Merged: 4/24/2026
Merged by: @profvjreddi

Base: dev ← Head: refactor/sidebar-single-source-of-truth

📝 Commits (1)

4c33d66 refactor(sidebar): consolidate to single source of truth

📊 Changes

5 files changed (+23 additions, -1531 deletions)

View changed files

📝 book/quarto/assets/styles/_base-styles.scss (+7 -59)
➖ book/quarto/assets/styles/style.scss (+0 -1351)
📝 kits/assets/styles/style.scss (+6 -41)
📝 labs/assets/styles/style.scss (+5 -40)
📝 mlsysim/docs/styles/style.scss (+5 -40)

📄 Description

Summary

The follow-up to PR #1514. That PR tightened the shared partial's values; this PR removes the duplicates so there's actually one place to edit going forward.

Before: five places defined .sidebar-navigation .sidebar-item a — the shared partial + four per-site copies, each keyed off a different accent variable (`$accent`, `$kits-accent`, `$colabs-accent`, `$mlsysim-accent`). Plus a 1350-line orphan `book/quarto/assets/styles/style.scss` that was referenced nowhere and carried yet another copy.

After: every site imports the shared partial once. Each site aliases `$accent` to its own site-accent (book via `themes/_theme-harvard`, kits/labs/mlsysim at line 19 of their `style.scss`), so the partial's accent-keyed hover/active states resolve to the right color per site without needing per-site rule copies.

Diff stat

 book/quarto/assets/styles/_base-styles.scss |   66 +-
 book/quarto/assets/styles/style.scss        | 1351 ---------------------------
 kits/assets/styles/style.scss               |   47 +-
 labs/assets/styles/style.scss               |   45 +-
 mlsysim/docs/styles/style.scss              |   45 +-
 5 files changed, 23 insertions(+), 1531 deletions(-)

Net −1508 lines, zero behavior change — the partial was already tightened to match these exact values in PR #1514, so removing the duplicates does nothing visible.

Orphan file deletion

`book/quarto/assets/styles/style.scss` was orphaned — the book's builds consume `style-vol1.scss` and `style-vol2.scss` (referenced in `book/quarto/config/_quarto-html-vol1.yml` line 175 and `_quarto-html-vol2.yml` line 178). I checked every `.yml`, `Makefile`, `.sh`, `.py`, `.js`, and `*.scss` file for references to `style.scss` — zero hits. Safe to delete. If something obscure turns up needing it, restore from `git show HEAD^:book/quarto/assets/styles/style.scss`.

Kept per-site

Two site-specific sidebar extras stay in their respective files because they're genuinely site-specific, not duplicated shape:

kits: `.sidebar-title` (teal underline styling) — not part of the shared partial's concerns
mlsysim: faint dividers between top-level sidebar sections — a visual choice only mlsysim wants

These are additive — they style different things from the shared rules.

Untouched

tinytorch: already imported the partial directly (was the site the whole redesign started from)
site (landing): imports the partial via `shared/styles/_ecosystem-base.scss`
slides, instructors: no sidebar rules before, no sidebar rules now

Test plan

Grep confirms the only `.sidebar-navigation .sidebar-item a` rule block in active SCSS is the shared partial
Grep confirms every Quarto site with a sidebar imports the partial
Pre-commit hooks pass
Post-merge: preview deploys on each site to confirm sidebars still render correctly (they should — same rules, same accent resolution, just via import instead of copy)

_{🔄 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/1515 **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:** `refactor/sidebar-single-source-of-truth` --- ### 📝 Commits (1) - [`4c33d66`](https://github.com/harvard-edge/cs249r_book/commit/4c33d66200e5f9e6397ca055c5ec6b0f95c58166) refactor(sidebar): consolidate to single source of truth ### 📊 Changes **5 files changed** (+23 additions, -1531 deletions) <details> <summary>View changed files</summary> 📝 `book/quarto/assets/styles/_base-styles.scss` (+7 -59) ➖ `book/quarto/assets/styles/style.scss` (+0 -1351) 📝 `kits/assets/styles/style.scss` (+6 -41) 📝 `labs/assets/styles/style.scss` (+5 -40) 📝 `mlsysim/docs/styles/style.scss` (+5 -40) </details> ### 📄 Description ## Summary The follow-up to PR #1514. That PR tightened the shared partial's values; this PR removes the duplicates so there's actually one place to edit going forward. **Before**: five places defined `.sidebar-navigation .sidebar-item a` — the shared partial + four per-site copies, each keyed off a different accent variable (\`\$accent\`, \`\$kits-accent\`, \`\$colabs-accent\`, \`\$mlsysim-accent\`). Plus a 1350-line orphan \`book/quarto/assets/styles/style.scss\` that was referenced nowhere and carried yet another copy. **After**: every site imports the shared partial once. Each site aliases \`\$accent\` to its own site-accent (book via \`themes/_theme-harvard\`, kits/labs/mlsysim at line 19 of their \`style.scss\`), so the partial's accent-keyed hover/active states resolve to the right color per site without needing per-site rule copies. ## Diff stat ``` book/quarto/assets/styles/_base-styles.scss | 66 +- book/quarto/assets/styles/style.scss | 1351 --------------------------- kits/assets/styles/style.scss | 47 +- labs/assets/styles/style.scss | 45 +- mlsysim/docs/styles/style.scss | 45 +- 5 files changed, 23 insertions(+), 1531 deletions(-) ``` **Net −1508 lines**, zero behavior change — the partial was already tightened to match these exact values in PR #1514, so removing the duplicates does nothing visible. ## Orphan file deletion \`book/quarto/assets/styles/style.scss\` was orphaned — the book's builds consume \`style-vol1.scss\` and \`style-vol2.scss\` (referenced in \`book/quarto/config/_quarto-html-vol1.yml\` line 175 and \`_quarto-html-vol2.yml\` line 178). I checked every \`*.yml\`, \`Makefile\`, \`*.sh\`, \`*.py\`, \`*.js\`, and \`*.scss\` file for references to \`style.scss\` — zero hits. Safe to delete. If something obscure turns up needing it, restore from \`git show HEAD^:book/quarto/assets/styles/style.scss\`. ## Kept per-site Two site-specific sidebar extras stay in their respective files because they're genuinely site-specific, not duplicated shape: - **kits**: \`.sidebar-title\` (teal underline styling) — not part of the shared partial's concerns - **mlsysim**: faint dividers between top-level sidebar sections — a visual choice only mlsysim wants These are additive — they style different things from the shared rules. ## Untouched - **tinytorch**: already imported the partial directly (was the site the whole redesign started from) - **site (landing)**: imports the partial via \`shared/styles/_ecosystem-base.scss\` - **slides, instructors**: no sidebar rules before, no sidebar rules now ## Test plan - [x] Grep confirms the only \`.sidebar-navigation .sidebar-item a\` rule block in active SCSS is the shared partial - [x] Grep confirms every Quarto site with a sidebar imports the partial - [x] Pre-commit hooks pass - [ ] Post-merge: preview deploys on each site to confirm sidebars still render correctly (they should — same rules, same accent resolution, just via import instead of copy) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

GiteaMirror added the pull-request label 2026-05-03 01:25:14 -05:00

GiteaMirror closed this issue

2026-05-03 01:25:17 -05:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/cs249r_book#9181