Skip to content

Commit 5d2190a

Browse files
committed
Apply final-review fixes: correct !important record, document styling convention, retire dead max-* artefacts, close anchors test gap
Fixes the four Important findings from the final whole-branch review: - .nav-github's !important is load-bearing (specificity, not layering); documented why. .footer-brand's !important was inert; dropped it and corrected the comment that had cited a false precedent. Corrected the Task 7/12 entries in deferred-findings.md to match. - Added a "Styling conventions" section to README.md recording the utilities-vs-scoped-CSS rule the branch actually followed. - Removed the dead --breakpoint-xs token, corrected Navbar's comment that claimed its media query was "mirrored with max-md: semantics" (the opposite is true), and noted in the plan that the max-* variant approach was rejected in favour of raw @media blocks. - extract.mjs now captures target/rel on anchors so the "target and label" test name matches what it checks. Also fixes a stale baseline line-range comment in index.astro (1298-1324 -> 1298-1325). 11/11 parity tests pass, astro build is clean, astro check reports 0 errors/warnings. Verified in Chrome that .footer-brand's computed color is unchanged (rgb(226, 232, 240)) with and without the !important.
1 parent 1147a1e commit 5d2190a

7 files changed

Lines changed: 37 additions & 13 deletions

File tree

README.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,19 @@ npm install
99
npm run dev
1010
```
1111

12+
## Styling conventions
13+
14+
- Use Tailwind utilities for simple layout and spacing on a component's outer
15+
elements.
16+
- Write a scoped `<style>` rule for anything with more than about four
17+
declarations, or that deserves a name worth reading.
18+
- Rules shared across multiple files go in `src/styles/global.css`, under
19+
`@layer components`.
20+
- Never set the same property both ways on the same element. Astro's scoped
21+
styles are unlayered, so they beat both `@layer base` and Tailwind
22+
utilities — a utility and a scoped rule targeting the same property do not
23+
resolve by specificity the way you'd expect; pick one per property.
24+
1225
## Test
1326

1427
```sh

docs/superpowers/plans/2026-08-23-astro-migration.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ For each component task the procedure is mechanical and identical:
7979
2. For each declaration, write the equivalent Tailwind utility on the element. Where no utility exists, use an arbitrary value (`bg-[rgba(10,14,23,0.85)]`, `w-[44px]`).
8080
3. The original stylesheet is **desktop-first**: the unprefixed rule is the desktop one and `@media (max-width: 768px)` / `@media (max-width: 480px)` blocks override it downward. **Mirror that structure exactly using Tailwind v4's `max-*` variants** — the desktop declaration stays unprefixed and the override becomes `max-md:` or `max-xs:`. Do **not** invert the stylesheet into mobile-first; inverting is the single most likely defect in this migration and buys nothing here.
8181

82+
*(Note, added during implementation: this `max-*` variant approach was rejected. All eight responsive overrides in the branch use raw `@media` blocks instead, because they match the baseline's inclusive `<=` boundaries exactly — see the next paragraph, and the comments in `Hero.astro` and `Navbar.astro`.)*
83+
8284
```
8385
/* baseline */ /* Tailwind */
8486
.stats-grid { grid-template-columns: repeat(3, 1fr) } grid-cols-3
@@ -88,6 +90,8 @@ For each component task the procedure is mechanical and identical:
8890
```
8991

9092
`max-md` is `< 48rem` and `max-xs` is `< 30rem`, against the baseline's `<= 768px` and `<= 480px`. The two differ only at exactly 768.00 and 480.00 CSS pixels. That is accepted.
93+
94+
*(Note, added during implementation: this off-by-one-pixel difference is not actually present in the branch — every responsive override was implemented as a raw `@media` block, not a `max-*` variant, precisely to hit the baseline's boundary exactly.)*
9195
4. Do not invent, round, or "improve" any value. Copy pixel and colour values exactly.
9296

9397
Two things Task 7 discovered the hard way, which every later component task inherits:
@@ -1173,6 +1177,8 @@ Markup: `tests/fixtures/baseline.html:844-864`. CSS: `tests/fixtures/baseline.ht
11731177
11741178
Keep the baseline's desktop-first structure: the horizontal nav stays unprefixed and the drawer override becomes `max-md:`. Do not invert to mobile-first.
11751179
1180+
*(Note, added during implementation: the drawer override was implemented as a raw `@media (max-width: 768px)` block, not `max-md:`, because `max-md:` excludes exactly 768px while the baseline's boundary is inclusive — see the comment in `Navbar.astro`.)*
1181+
11761182
- [ ] **Step 2: Write `src/components/Navbar.astro`**
11771183
11781184
```astro

src/components/Footer.astro

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,15 +51,13 @@ const social = [
5151
gap: 1.5rem;
5252
}
5353

54-
/* .footer-brand, baseline.html:677-681. Kept `!important` as a faithful
55-
transcription, matching Navbar's .nav-github precedent (baseline.html:
56-
172-182) — it is redundant now that Astro's unlayered scoped styles
57-
already beat `@layer base`'s `a { color }`, but Task 6/7 chose to keep
58-
it there rather than drop it, and this component follows that choice
59-
for consistency rather than re-litigating it per component. */
54+
/* .footer-brand, baseline.html:677-681. Unlayered scoped rule beats
55+
global.css's @layer base `a { color }`, so no !important needed here —
56+
unlike .nav-github (Navbar.astro), which competes with a
57+
higher-specificity sibling rule (.nav-links a) and does need it. */
6058
.footer-brand {
6159
font-weight: 700;
62-
color: var(--color-text-primary) !important;
60+
color: var(--color-text-primary);
6361
font-size: 1.05rem;
6462
}
6563

src/components/Navbar.astro

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,11 @@ import { Icon } from 'astro-icon/components';
7474
color: var(--color-text-primary);
7575
}
7676

77-
/* .nav-github, baseline.html:172-182 */
77+
/* .nav-github, baseline.html:172-182. !important is required here:
78+
.nav-links a (0,4,1) outranks .nav-github (0,2,0), both unlayered, so
79+
specificity — not layering — decides. Verified: removing it renders
80+
#94a3b8 (--color-text-secondary) instead of #e2e8f0
81+
(--color-text-primary). Do not drop it. */
7882
.nav-github {
7983
display: inline-flex;
8084
align-items: center;
@@ -107,8 +111,11 @@ import { Icon } from 'astro-icon/components';
107111
cursor: pointer;
108112
}
109113

110-
/* Desktop-first: the baseline's @media (max-width: 768px) override,
111-
baseline.html:705-726, mirrored with max-md: semantics. */
114+
/* Desktop-first: baseline's @media (max-width: 768px) override,
115+
baseline.html:705-726. A real media query is used, not Tailwind's
116+
max-md: variant: max-md: compiles to (width < 768px), which excludes
117+
exactly 768px, while the baseline's media query includes it. Matches
118+
the reasoning in Hero.astro. */
112119
@media (max-width: 768px) {
113120
.nav-links {
114121
display: none;

src/pages/index.astro

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ import { libraries } from '../data/libraries';
5858
</div>
5959
</section>
6060

61-
<!-- Install, baseline.html:1298-1324 -->
61+
<!-- Install, baseline.html:1298-1325 -->
6262
<Install />
6363

6464
<!-- Footer, baseline.html:1327-1343 -->

src/styles/global.css

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@
2323
--radius-card: 12px;
2424
--radius-chip: 8px;
2525

26-
--breakpoint-xs: 30rem;
27-
2826
--font-sans: 'Inter Variable', -apple-system, BlinkMacSystemFont, 'Segoe UI', sans-serif;
2927
--font-mono: 'JetBrains Mono Variable', 'Fira Code', ui-monospace, monospace;
3028
}

tests/extract.mjs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ export function extractSnapshot(html) {
4343
anchors: body.querySelectorAll('a').map((a) => ({
4444
href: normPath(a.getAttribute('href')),
4545
text: norm(a.text),
46+
target: a.getAttribute('target') ?? null,
47+
rel: a.getAttribute('rel') ?? null,
4648
})),
4749
images: body.querySelectorAll('img').map((img) => ({
4850
src: normPath(img.getAttribute('src')),

0 commit comments

Comments
 (0)