feat(ui): Batch A — hierarchy + payoff surface#23
Conversation
…/export-core changes) Pure UI reorganization. Regroup left rail into Identity → Colors → Typography → Spacing → Advanced (collapsed by default, native <details>). Frame preview with chrome header + shadowed canvas. Swap widget-card opacity-dim for neutral ghost + primary ring + check badge on selected. Rebuild export panel as payoff: chip tabs with .ext sub-line, per-format description, Download as primary CTA, Copy as ghost. Align app shell rhythm: sidebar 300px, 12px radii, consistent cards. All 267 tests still pass. Schema, persistence, and export formatters untouched. 2-color Simple mode derivation deferred to Batch B. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Code Review
This pull request implements a comprehensive UI reorganization and visual polish for the theme-forge application, focusing on layout rhythm, component hierarchy, and accessibility. Key updates include a redesigned export panel with format descriptions, enhanced widget selection states with visual badges, and a regrouped sidebar that introduces a collapsible 'Advanced' section for shadows and radii. However, a high-severity issue was identified in the new Collapsible component: using the 'open' attribute instead of 'defaultOpen' on the native details element makes it a controlled component without state management, causing the section to snap shut during every re-render (e.g., on every keystroke in the editor).
| children: ReactNode | ||
| }) { | ||
| return ( | ||
| <details className={styles.collapsible} open={defaultOpen}> |
There was a problem hiding this comment.
The Collapsible component uses the open attribute on the <details> element, which makes it a controlled component in React. Since it is being passed a literal false (via defaultOpen={false} on line 372) and there is no state management to update this value when the user interacts with the component, the section will forcefully collapse every time the parent ThemeEditor re-renders. This happens on every keystroke in any of the editor's inputs, making the "Advanced" section practically unusable as it snaps shut while typing.
To fix this while keeping the component uncontrolled, use the defaultOpen attribute instead of open.
| <details className={styles.collapsible} open={defaultOpen}> | |
| <details className={styles.collapsible} defaultOpen={defaultOpen}> |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d8d503f5c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| children: ReactNode | ||
| }) { | ||
| return ( | ||
| <details className={styles.collapsible} open={defaultOpen}> |
There was a problem hiding this comment.
Preserve Advanced panel open state across re-renders
Using open={defaultOpen} turns the <details> into a controlled element that is always forced back to false here, so once a user opens Advanced, any subsequent ThemeEditor re-render (for example typing in a shadow textarea or radius input) collapses the panel again. This makes editing advanced fields frustrating and effectively breaks the new collapsible workflow; the component should treat open state as uncontrolled after initial mount or manage it with real state.
Useful? React with 👍 / 👎.
Summary
Pure UI reorganization + visual polish. No schema, persistence, export-core, or product-boundary changes. All 267 existing tests remain green.
What changed
`).
Constraints honored
Deferred
Test plan
🤖 Generated with Claude Code