Migrate StarterPackCatalogModal to design tokens

[Chris0Jeky]

Summary

• Replaced all hardcoded Tailwind color classes (bg-white, text-gray-*, border-gray-*, bg-blue-*, bg-red-*, bg-green-*, bg-amber-*, etc.) in StarterPackCatalogModal.vue with scoped CSS classes that reference --td-* design tokens.
• Added a <style scoped> block with semantic class names (sp-panel, sp-tab, sp-btn-primary, sp-tone-success, sp-callout-error, etc.) that map to design tokens from design-tokens.css.
• The modal now correctly respects dark/light theme switching via data-theme.
• Computed status-indicator classes (outcomeSummaryToneClass, conflictSeverityBadgeClass) now return semantic class names instead of Tailwind color utilities.

Closes #612

Test plan

• npm run typecheck passes
• npm run build passes (typecheck + vite build)
• Verified zero remaining hardcoded Tailwind color classes via grep
• Visual verification: open Starter Packs modal in both dark and light themes
• Verify status indicators (success/error/warning/info tones) render correctly
• Verify tab active state uses primary accent color
• Verify pack card selection highlight uses primary accent

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[Chris0Jeky]

Self-review of #640

Completeness check

• Zero hardcoded Tailwind color classes remain -- verified via grep for bg-white, text-gray-*, border-gray-*, bg-blue-*, bg-red-*, bg-green-*, bg-amber-*. All clean.
• Both tabs (Catalog and JSON Import) are migrated. The import tab mirrors the same pattern as the catalog tab.
• All 4 computed style functions (outcomeSummaryToneClass, importOutcomeSummaryToneClass, conflictSeverityBadgeClass) return semantic class names.

Token mapping correctness

Old Tailwind	New token	Notes
bg-white	--td-surface-primary	Panel background
bg-gray-50	--td-surface-secondary	Inset/empty-state backgrounds
text-gray-900 / text-gray-800	--td-text-primary	Headings, strong text
text-gray-600 / text-gray-700	--td-text-secondary	Body text, labels
text-gray-500 / text-gray-400	--td-text-muted / --td-text-tertiary	Hints, close button
border-gray-200 / border-gray-300	--td-border-default	Standard borders
border-gray-100	--td-border-ghost	Subtle list borders
bg-blue-600	--td-color-primary	Primary action button
border-blue-500 / text-blue-600	--td-color-primary	Active tab, selected card
bg-blue-50	--td-color-primary-light	Selected card highlight
bg-green-100 text-green-700	--td-color-success-light / --td-color-success	Success tone
bg-red-100 text-red-700	--td-color-error-light / --td-color-error	Error tone
bg-amber-100 text-amber-800	--td-color-warning-light / --td-color-warning	Warning tone
bg-blue-100 text-blue-700	--td-color-info-light / --td-color-info	Info tone

Potential concerns

1. focus:ring-2 focus:ring-blue-500 on inputs was replaced with box-shadow: var(--td-focus-ring) in .sp-input:focus. This uses the ember-red focus ring consistent with the rest of the app, rather than blue. This is intentional alignment with the design system.
2. Disabled primary button now uses --td-surface-bright + --td-text-muted instead of bg-gray-400. This should still read as "disabled" visually in both themes.
3. No layout or structural changes -- only color/theme classes were touched. Spacing, sizing, and grid behavior are preserved via Tailwind utility classes.

Build verification

• npm run typecheck -- pass
• npm run build (typecheck + vite production build) -- pass
• No new dependencies or token additions needed; all referenced tokens already exist in design-tokens.css.

[gemini-code-assist]

Code Review

This pull request migrates the StarterPackCatalogModal.vue component from hardcoded Tailwind utility classes to a centralized design token system using semantic CSS variables. The changes include replacing inline classes with custom scoped classes (e.g., sp-panel, sp-btn-primary) and adding a comprehensive <style> block. Feedback focuses on improving the consistency and accessibility of these new styles, specifically regarding hardcoded backdrop colors, low-contrast borders on error boxes, redundant class definitions, and accessibility concerns for disabled button states.

[gemini-code-assist]

The backdrop background uses a hardcoded rgba(0, 0, 0, 0.5) value. To maintain consistency with the design token migration, this should reference a semantic token (e.g., --td-backdrop-bg). If a specific token is missing from the global design tokens, consider defining one to ensure the backdrop can be themed appropriately.

[gemini-code-assist]

Using var(--td-color-error-light) for the border results in a very low-contrast (15% opacity) line that is difficult to see. For error and warning boxes, it is generally better to use the solid semantic color for the border to ensure the region is clearly delimited. This also applies to .sp-callout-error and .sp-callout-warning below.

  border: 1px solid var(--td-color-error);

[gemini-code-assist]

The .sp-callout-error class is identical to .sp-error-box defined earlier. To improve maintainability and reduce CSS redundancy, consider consolidating these into a shared base class or using a single semantic class for both purposes.

[gemini-code-assist]

The combination of var(--td-surface-bright) background and var(--td-text-muted) text for disabled primary buttons results in a low contrast ratio (approx 2.5:1 in dark mode). While disabled states are exempt from standard WCAG contrast requirements, this may still be difficult to read. Consider using a slightly more distinct color pair for better accessibility.

[Chris0Jeky]

Adversarial Review: StarterPackCatalogModal Design Token Migration

Overall Assessment

The migration is thorough and well-structured. All hardcoded Tailwind color utilities (bg-gray-*, text-gray-*, border-gray-*, bg-blue-*, bg-red-*, bg-green-*, bg-amber-*) have been replaced. No stray color classes remain in the template. The scoped <style> block is well-organized with clear section comments. That said, I have several findings:

---

1. Hardcoded backdrop color (Low)

.sp-backdrop {
  background: rgba(0, 0, 0, 0.5);
}

This is the only remaining hardcoded color in the file. Every other color uses --td-* tokens. While a black backdrop works in both themes, for full token consistency this should reference a token or at minimum use a CSS custom property. In light mode, a slightly softer overlay might be more appropriate.

Suggestion: Consider background: var(--td-backdrop, rgba(0, 0, 0, 0.5)) with a token defined in design-tokens.css, or at least add a comment explaining why this is intentionally hardcoded.

---

2. --td-color-info aliases --td-color-primary (Medium)

In the design tokens file:

• Dark: --td-color-info: #ffb3ae (same as --td-color-primary)
• Light: --td-color-info: var(--_td-light-accent) (same as --td-color-primary)

The sp-tone-info class (used for dry-run results) will look identical to primary/accent elements. This means a dry-run result badge is visually indistinguishable from the "Apply Starter Pack" button's accent color, which breaks the semantic distinction the migration is trying to create. Users won't be able to tell at a glance that info != primary action.

This is a token design issue rather than a migration bug, but the migration surfaces it.

---

3. Hover state on .sp-pack-card uses --td-border-ghost (Low)

.sp-pack-card:hover {
  border-color: var(--td-border-ghost);
  background: var(--td-surface-tertiary);
}

--td-border-ghost is defined as rgba(91, 64, 62, 0.1) (dark) / rgba(28, 25, 23, 0.06) (light) — this is less visible than --td-border-default. On hover, the border becomes lighter than the resting state, which is the opposite of normal hover affordance (where the border should become more prominent). The original Tailwind code used hover:border-gray-300 which was darker than border-gray-200.

Suggestion: Use a token with more opacity for hover, e.g., --td-border-default or a dedicated --td-border-hover.

---

4. Computed style functions — all 4 migrated correctly (Good)

The 4 class-returning functions are:

1. outcomeSummaryToneClass — migrated to sp-tone-* classes
2. importOutcomeSummaryToneClass — migrated to sp-tone-* classes
3. conflictSeverityBadgeClass — migrated to sp-tone-warning / sp-tone-error
4. Template ternaries for callout/conflict sections — migrated to sp-callout-*, sp-text-error, sp-text-warning

All branches are covered. No hardcoded colors remain in any computed style path.

---

5. Dark mode correctness (Good with caveat)

The token file has both :root (dark) and [data-theme="light"] overrides for every token used. The migration will render correctly in both themes.

Caveat: The sp-error-box uses a fallback pattern:

border: 1px solid var(--td-color-error-light, var(--td-color-error));

This fallback is good defensive CSS. However, sp-callout-error and sp-callout-warning use the same pattern, but sp-tone-error / sp-tone-success / etc. do not use fallbacks. If any -light token were ever removed, the tone badges would silently lose their background. Consider adding fallbacks consistently or not at all.

---

6. sp-btn-primary:disabled contrast concern (Medium)

.sp-btn-primary:disabled {
  background: var(--td-surface-bright);
  color: var(--td-text-muted);
}

In dark mode: --td-surface-bright: #3a3939 with --td-text-muted: rgba(229, 226, 225, 0.6). The effective text color is approximately #9b9998 on #3a3939 — contrast ratio ~2.7:1, which fails WCAG AA for normal text (requires 4.5:1). The original disabled:bg-gray-400 with white text had a similar issue, but this is a chance to fix it.

In light mode: --td-surface-bright: #c9c6c4 with --td-text-muted: rgba(28, 25, 23, 0.55). Effective text ~#858280 on #c9c6c4 — contrast ratio ~1.7:1, which also fails WCAG AA.

---

7. Missing disabled state on sp-btn-secondary (Low)

The sp-btn-primary has explicit :disabled styling, but sp-btn-secondary does not. The template still uses disabled:cursor-not-allowed disabled:opacity-60 via Tailwind utilities on secondary buttons, so it works, but the approach is inconsistent — primary buttons use scoped CSS for disabled state while secondary buttons use Tailwind utilities.

---

8. sp-tab inactive hover goes to --td-text-secondary but active uses --td-color-primary (Nit)

The inactive tab color is --td-text-muted, hover is --td-text-secondary, and active is --td-color-primary. In the original code, inactive was text-gray-500 and hover was text-gray-700. The new mapping is semantically correct, just confirming this is intentional — the jump from muted to primary on click is larger than the original gray-500 to blue-600 jump.

---

Summary

Finding	Severity	Action needed?
Hardcoded backdrop rgba(0,0,0,0.5)	Low	Optional tokenize
Info token = Primary token (no visual distinction)	Medium	Token design issue
Pack card hover border gets lighter	Low	Use stronger border token
Computed styles fully migrated	--	None (good)
Dark/light mode coverage	--	Good
Disabled primary button contrast	Medium	Fix for accessibility
Inconsistent disabled styling approach	Low	Standardize
Tab hover/active jump	Nit	Intentional check

No blocking issues, but findings #3 (inverted hover border) and #6 (disabled button contrast) are worth addressing before merge.