Skip to content

Apply Dixie refresh style deltas - #33

Merged
dpup merged 2 commits into
mainfrom
dixie-style-deltas
Jul 18, 2026
Merged

Apply Dixie refresh style deltas#33
dpup merged 2 commits into
mainfrom
dixie-style-deltas

Conversation

@dpup

@dpup dpup commented Jul 18, 2026

Copy link
Copy Markdown
Member

Style + layout changes to match the Dixie session + dashboard mocks, plus a couple of follow-up tunes.

Changes

# Change File(s) Risk
1 Light-mode card border theme.css .gp-card low
2 Pill TabBar → segmented control TabBar.tsx low
3 Orange-tinted active nav item SidebarLink.tsx / SidebarItem.tsx low
4 Always-navy sidebar chrome Sidebar.tsx + sub-components brand decision
5 Cream page background in light mode Layout.tsx / ContentPane.tsx low
6 Non-wrapping ProportionChart legend ProportionChart.tsx low

1 — Card light-mode border

.gp-card gets a hairline border-gray-200 in light mode for cleaner separation on the cream page (dark unchanged).

2 — Pill TabBar → segmented control

The pill TabBar drops the uppercase/tracking treatment, sizes up (px-5 py-1.5 text-sm), and gets a container border so it reads as a modern segmented control. Active state unchanged. (This doubles as the "button group in pill mode" control — session sub-nav, dashboard time-range switch.)

3 — Orange-tinted active nav item

The active nav row now carries the orange brand accent. Applied to both SidebarLink (compound API) and SidebarItem (the items-prop API) so the two code paths stay visually identical, plus softer rounded-lg corners.

4 — Always-navy sidebar chrome (brand decision)

The sidebar is chrome, so it now renders navy in both light and dark mode. Reworked SidebarLink, SidebarItem, SidebarUser, SidebarSearch, SidebarSection, SidebarGroup, and the collapse toggle to their on-navy treatment (collapsing the redundant light dark: pairs into a single navy treatment) so text stays readable on navy in light mode.

5 — Cream page background in light mode

The main content area fell back to bg-gray-50 in light mode; it now uses the cream brand token (Layout.tsx ×2 page wrappers + ContentPane.tsx). Dark (navy-950) unchanged. Dropdown-menu hover states keep bg-gray-50 (chrome).

6 — Non-wrapping ProportionChart legend

Added a legendColumns?: number prop (defaults to the visible segment count). The legend now lays out in a single non-wrapping row of that many columns, so a trailing item can't become a lone "widow" on a second row. Labels/values truncate. Note: this drops the old responsive grid-cols-2 sm:grid-cols-5 — a wide legend stays one row on narrow viewports rather than reflowing.

Playground

  • SidebarDemo note updated to reflect always-navy chrome.
  • ProportionChartDemo gains a "Fixed legend columns (no widow)" example + usage note.

Verification

  • npm run build
  • npm run typecheck
  • npm run lint
  • npm run test:run ✓ (63 tests)

🤖 Generated with Claude Code

- Card: add hairline border in light mode (dark unchanged)
- Pill TabBar: read as a segmented control — drop uppercase/tracking,
  size up, add container border
- Sidebar: active nav row carries the orange brand accent (SidebarLink
  and SidebarItem), softer rounded-lg corners
- Sidebar chrome is now always navy in both light and dark mode; reworked
  SidebarLink, SidebarItem, SidebarUser, SidebarSearch, SidebarSection,
  SidebarGroup, and the collapse toggle to their on-navy treatment

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Preview deployed! 🚀

View the playground preview at: https://thegpvc.github.io/gp-ui/pr-33/

Last updated: 2026-07-18T05:26:06.045Z

@claude

claude Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Review: Apply Dixie refresh style deltas (#33)

Reviewed the diff via gh pr diff 33 against this repo's CLAUDE.md/DESIGN_GUIDE.md conventions. Overall this is a well-scoped, low-risk visual-only PR — no logic/prop changes, and the four deltas match their stated intent. A few notes below.

Overview

  • Delta 1.gp-card gains a light-mode hairline border (border-gray-200), dark unchanged.
  • Delta 2TabBar pill variant loses the uppercase/tracking treatment, sizes up, and gets a container border → reads as a segmented control.
  • Delta 3 — Active sidebar row gets an orange accent + rounded-lg, applied consistently to both SidebarLink (compound API) and SidebarItem (items-prop API).
  • Delta 4 — Sidebar chrome (Sidebar, SidebarLink, SidebarItem, SidebarUser, SidebarSearch, SidebarSection, SidebarGroup) drops dark: pairs in favor of a single always-navy treatment.

Code quality / conventions

  • Delta 4's approach (hardcoding the navy classes instead of light dark:navy pairs) is the right call here, not a violation of the "class dark:class" rule — since the sidebar surface is now always navy regardless of app theme, a dark: variant would be dead weight. This matches the "chrome = navy-850" tier from DESIGN_GUIDE.md.
  • The SidebarUser email color change (text-navy-500 dark:text-navy-300text-navy-300) correctly fixes a preexisting navy-500-on-dark contrast smell called out in CLAUDE.md ("Never use navy-500 for readable text on dark").
  • Playground (SidebarDemo.tsx) was updated to describe the new always-navy/orange-accent behavior — good, matches the "update the playground" requirement.

Potential issues

  1. Arbitrary opacity value, duplicated (SidebarLink.tsx:39, SidebarItem.tsx:27): bg-orange-500/[0.14] is a hand-picked arbitrary value, not one of the standard Tailwind steps (/10, /20, …) already used elsewhere in this codebase (e.g. theme.css:186 uses dark:bg-orange-500/10). Per CLAUDE.md's token-first rule ("if a value will be used twice, put it in theme.css under @theme... inline arbitrary classes are a review smell"), this should either become a token/utility class or snap to the nearest standard step (/10 or /15) so it isn't a magic number duplicated in two files that can drift out of sync.
  2. Duplicated active-state class string across SidebarLink.tsx and SidebarItem.tsx — the PR body explains this is intentional (keeping the two code paths visually identical), but it also means every future tweak to the active-state treatment needs to be made in two places by hand. Worth considering a small shared constant (e.g. SIDEBAR_ACTIVE_CLASSES) per the "avoid duplication" guideline — not blocking, just a maintenance risk given this delta already required touching both files to stay in sync.
  3. Minor: redundant dark:border in theme.css.gp-card's dark @apply still includes dark:border even though the base rule now unconditionally applies border (delta 1). Harmless (idempotent), but could be dropped for clarity now that the border is present in both modes.

Performance

No concerns — purely class-name/CSS changes, no new renders, no new dependencies.

Security

No concerns — no user input, no new DOM injection, no new external calls.

Test coverage

No test files exist for Sidebar/TabBar today, and this PR doesn't add any. Since all four deltas are purely visual (no new props, no new branching logic beyond the existing isActive conditional), this is consistent with CLAUDE.md's "components don't need exhaustive tests" guidance — but if a SidebarLink.test.tsx ever gets written, asserting the active-state class output (delta 3) would be a good first contract test given it's now spread across two files.

Verdict

Solid, well-scoped style PR. Recommend snapping the /[0.14] opacity to a standard step or token before merge (#1); the rest are non-blocking suggestions.

- Layout and ContentPane main-area background falls back to cream in
  light mode (was bg-gray-50); dark unchanged
- ProportionChart: add legendColumns prop; legend now lays out in a
  single non-wrapping row (defaults to the visible segment count) so a
  trailing item can't become a lone widow. Labels/values truncate.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@dpup
dpup merged commit 78f36fb into main Jul 18, 2026
4 checks passed
@dpup
dpup deleted the dixie-style-deltas branch July 18, 2026 05:27
@github-actions

Copy link
Copy Markdown
Contributor

🧹 Preview deployment has been cleaned up.

github-actions Bot added a commit that referenced this pull request Jul 18, 2026
@claude

claude Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Review

Overall this is a clean, well-scoped style PR that follows the repo's conventions well (three-tier dark hierarchy, muted-text contrast rule, token usage). A few things worth a look before merge:

🐛 Likely bug: active nav item loses its orange accent on hover

In both SidebarItem.tsx and SidebarLink.tsx:

const baseClasses = cn(
  "flex items-center gap-3 px-2 py-2 rounded-lg text-sm font-medium transition-colors w-full",
  "text-navy-300 hover:text-white hover:bg-navy-700",
  isActive && "bg-orange-500/[0.14] text-orange-300 hover:text-orange-300"
);

hover:bg-navy-700 is applied unconditionally, and the active branch never overrides it with its own hover:bg-*. Since a :hover pseudo-class utility has higher CSS specificity than a plain utility of the same property, hover:bg-navy-700 will win over the static bg-orange-500/[0.14] whenever the active item is hovered — so the orange-tinted background disappears and gets replaced by plain navy-700, even though the text stays orange-300. Net effect: hovering the currently-active row makes it look like it lost its active state (mismatched bg/text) for the duration of the hover.

The previous code avoided this because active and hover resolved to the same color in dark mode (dark:bg-navy-700 / dark:hover:bg-navy-700), so the swap was invisible. Now that active uses a distinct orange tint, the two states visibly conflict. Worth adding something like isActive && "bg-orange-500/[0.14] text-orange-300 hover:bg-orange-500/[0.20] hover:text-orange-300" so the active row has its own hover treatment.

Breaking visual change with no changelog entry

Delta 4 makes Sidebar (and its sub-components) permanently navy regardless of light/dark mode — this is a real, app-wide visual change for any consumer currently seeing a white sidebar in light mode, not just a "brand decision" note in the PR description. The repo's CHANGELOG.md has a [0.3.0] entry with an explicit "Changed" section calling out similar surface/contrast shifts — this PR doesn't add an entry, and package.json version is untouched. Given consumers scan dist/ classes directly, this will silently change rendered UI on upgrade. Might be worth a changelog line (and maybe a minor version bump) so consuming apps aren't surprised.

Also worth double-checking against the CLAUDE.md guidance that mode="dark" is the documented escape hatch for "elements on fixed-dark surfaces inside a light app" — here the dark: variants were stripped outright rather than exposed via a prop. If the intent is genuinely "sidebar is chrome, always navy, no per-instance override," hardcoding is reasonable and simpler than a mode prop nobody would ever set to "light". But if any consumer app relies on a light sidebar today, there's no way to opt back in.

ProportionChart legend: no responsive fallback

The legend grid moved from grid-cols-2 sm:grid-cols-5 (wraps responsively) to a fixed repeat(N, minmax(0,1fr)) that never wraps. That's the intended fix for the "widow" problem, but it also removes all viewport-based responsiveness: with many segments (e.g. 8+) on a narrow/mobile viewport, every legend cell gets squeezed into a sliver, and label/value get truncated to nothing. Might be worth capping legendColumns at some viewport-aware max, or documenting that callers with many segments should pass a smaller legendColumns and accept wrapping some other way.

Minor: if a caller passes legendColumns={0} (or a negative number), repeat(0, minmax(0,1fr)) is invalid and the legend silently collapses. Not a real-world scenario given the default, but worth a Math.max(1, ...) guard if you want to be defensive against misuse.

Test coverage

legendColumns is new, testable behavior (per the repo's own guidance: "new behavior should be locked down") but ProportionChart.test.tsx wasn't updated — no test asserts the gridTemplateColumns style for a custom value or the default-to-segment-count behavior. Given the existing legend tests in that file, adding 1-2 cases would be cheap and would have caught the columns-per-viewport question above during review.

Nit

theme.css .gp-card: the base rule now applies border border-gray-200 unconditionally, so the dark override's dark:border is redundant (border-width/style is already set; only dark:border-navy-700 for color is needed). Harmless, just a tiny duplication.

What looks good

  • Token usage is consistent (bg-cream, navy-850, etc. all exist in theme.css — no arbitrary bg-[#...] smells).
  • SidebarUser secondary text correctly moved to navy-300 per the muted-text contrast rule in CLAUDE.md.
  • TabBar pill variant change (drop uppercase/tracking, size up, add container border) is applied consistently and playground/docs example (legendColumns={5}) was updated alongside the prop.
  • Build/typecheck/lint all reported green in the PR description.

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant