Skip to content
This repository was archived by the owner on Aug 17, 2026. It is now read-only.

fix(dev-portal): restore stroke=currentColor on sidebar icons (closes #48) - #54

Merged
pascal-klesse merged 1 commit into
mainfrom
fix-issue-048-sidebar-icons
May 2, 2026
Merged

fix(dev-portal): restore stroke=currentColor on sidebar icons (closes #48)#54
pascal-klesse merged 1 commit into
mainfrom
fix-issue-048-sidebar-icons

Conversation

@pascal-klesse

Copy link
Copy Markdown
Member

Summary

  • Restores visibility of dev-portal sidebar icons that broke during the shadcn migration in feat(dev-portal): migrate SPA to shadcn/ui + tailwind 4 (closes #40) #41 — every inline <svg> was rendered with fill="none" but no stroke, so the <path> / <line> / <circle> elements painted nothing.
  • Adds four Lucide-style stroke attributes to the shared COMMON props in src/core/dx/clients/layout/icons.tsx: stroke="currentColor", strokeWidth={1.75}, strokeLinecap="round", strokeLinejoin="round". The wrapper text-… utilities on AdminShell already supply the colour, including the active-state accent (text-accent).
  • Pins the regression in tests/stories/dev-portal-pages.story.test.ts so a future refactor can't silently drop the stroke attributes again.

Why this works

The legacy CSS rule that compensated for the missing stroke (.admin-nav__icon svg path { stroke: currentColor; } in the deleted admin-layout.css) is gone. Adding the stroke attributes directly on the SVG closes the gap and keeps icon colour inheritance flowing through currentColor — exactly what text-accent on the active <NavLink> already drives.

BRAND_LOGO already declared its strokes inline and was unaffected.

Acceptance criteria (Issue #48)

  • COMMON props extended with stroke="currentColor" + strokeWidth={1.75} + strokeLinecap="round" + strokeLinejoin="round"
  • Story-test in tests/stories/dev-portal-pages.story.test.ts asserts the four stroke attributes are present
  • Active-state (lime accent) propagates via currentColor inheritance — no callsite changes needed
  • Visual smoke test: sidebar icons render visibly across all sections (human reviewer)

Test plan

  • bun run lint — clean
  • bun run format — clean
  • bun run test:types — clean
  • bun run test:unit — 153/153 passed
  • bun run test:e2e — 2412/2412 passed (story-test pin transitioned RED → GREEN)
  • bun run test:coverage — 91.09% statements / 81.88% branches / 92.96% lines, thresholds met
  • bun run build:dev-portal — 57 artifacts (971.7 kB)
  • bun run build — full server build clean
  • Visual smoke test on bun run dev (human reviewer): every sidebar section icon visible, active-state lime tint correct

Out of scope

  • Migration to lucide-react (breaking — icon names + paths change)
  • Adding new icons for pages without one

🤖 Generated with Claude Code

After the shadcn migration in PR #41, the sidebar icons rendered
invisibly because `admin-layout.css` (which carried
`.admin-nav__icon svg path { stroke: currentColor; }`) was deleted —
leaving the inline SVGs with `fill="none"` but no stroke directive.

Add the four Lucide-style stroke attributes to the shared `COMMON`
props on `icons.tsx` so every `<path>/<line>/<circle>` paints with the
inherited `currentColor`. The wrapper `text-…` utilities on
`AdminShell` already supply that colour, including the active-state
accent — no callsite changes needed.

Pin the regression in `tests/stories/dev-portal-pages.story.test.ts`
so a future agent can't silently drop the stroke attributes again.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@pascal-klesse
pascal-klesse force-pushed the fix-issue-048-sidebar-icons branch from 1b407e9 to 20ef4ad Compare May 2, 2026 21:04
@pascal-klesse
pascal-klesse merged commit 09fe832 into main May 2, 2026
10 checks passed
@pascal-klesse
pascal-klesse deleted the fix-issue-048-sidebar-icons branch May 4, 2026 12:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant