Add Opt-in to exported CSS Vars and clean generated CSS code #200
Conversation
|
@llanesluis is attempting to deploy a commit to the tweakcn OSS program Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdded a UI preference for including font variables, threaded that preference through theme and Tailwind code generators, and refactored the code panel to use controlled tabs with constants and auto-switching when Tailwind version changes. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CodePanel
participant PreferencesStore
participant ThemeGenerator
User->>CodePanel: Open Preferences Popover
Note right of CodePanel #EFEFEF: Popover contains "Include font variables" Switch
User->>CodePanel: Toggle Switch
CodePanel->>PreferencesStore: setIncludeFontVariables(value)
PreferencesStore-->>CodePanel: includeFontVariables updated
CodePanel->>ThemeGenerator: generateThemeCode(..., { includeFontVariables })
ThemeGenerator-->>CodePanel: Generated CSS / Tailwind config (with/without font vars)
CodePanel-->>User: Render updated code tab
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/editor/code-panel.tsx (1)
218-221: Consider improving the explanatory text.The current text "If you handle fonts separately, turn this OFF" could be more specific about what "handling fonts separately" means.
- <span className="text-muted-foreground text-xs text-pretty"> - If you handle fonts separately, turn this OFF. - </span> + <span className="text-muted-foreground text-xs text-pretty"> + Disable if you manage font definitions outside of this theme (e.g., in a separate CSS file or via @import). + </span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
components/editor/code-panel.tsx(6 hunks)store/preferences-store.ts(4 hunks)utils/theme-style-generator.ts(5 hunks)
🔇 Additional comments (10)
store/preferences-store.ts (1)
16-16: LGTM! Clean implementation of the new preference.The new
includeFontVariablespreference follows the established patterns in the store, with proper TypeScript typing, sensible default value, and consistent setter implementation.Also applies to: 22-22, 34-34, 52-54
components/editor/code-panel.tsx (3)
37-37: LGTM! Proper integration with the preferences store.The component correctly reads the
includeFontVariablesstate and passes it to the theme code generator. The integration follows React and Zustand best practices.Also applies to: 41-41, 50-52
174-231: Well-structured UI enhancement with good UX.The restructured layout effectively groups related controls and the new preferences popover provides clear context for the font variables option. The explanatory text helps users understand when to disable this feature.
138-138: Good responsive design improvements.The className changes improve button sizing consistency and add proper responsive width handling.
Also applies to: 248-248
utils/theme-style-generator.ts (6)
71-73: Well-defined type for preferences.The
GenerateVarsPreferencestype is appropriately named and uses optional properties, allowing for future extensibility.
75-82: Proper implementation of conditional font variables.The function correctly destructures the preferences with a sensible default and conditionally generates font variables based on the user preference.
Also applies to: 85-85
119-129: Good refactoring of tracking variables generation.The new
generateInlineTrackingVariablesfunction properly handles the conditional generation based on whether letter-spacing is "0em", avoiding unnecessary CSS when not needed.
131-201: Consistent implementation of preferences in Tailwind v4 theme.The function properly handles the
includeFontVariablespreference and maintains consistency with the main theme generation logic.
203-208: Clean function signature extension.The main
generateThemeCodefunction properly accepts the new preferences parameter with appropriate defaults and passes it through to the generation functions consistently.Also applies to: 220-223
100-116: All Clear: No Dark-Mode Dependencies FoundA repository-wide search in CSS/SCSS/TS/TSX for
--font-*,--radius-*,--tracking-*, and--spacing-*under a.darkcontext returned zero matches. Removing these overrides in dark mode will not impact any existing themes.
|
Superb change! |
|
Sure I'll merge main and check for conflicts |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
utils/theme-style-generator.ts (1)
203-290: Functional implementation with room for improvement.The conditional font family inclusion works correctly. The string-based approach for building the config is functional, though consider using object composition for better maintainability in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/editor/code-panel.tsx(8 hunks)utils/theme-style-generator.ts(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/editor/code-panel.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
utils/theme-style-generator.ts (4)
types/theme.ts (1)
ThemeStyles(72-72)types/editor.ts (1)
ThemeEditorState(18-27)types/index.ts (1)
ColorFormat(53-53)utils/color-converter.ts (1)
colorFormatter(14-46)
🔇 Additional comments (7)
utils/theme-style-generator.ts (7)
2-6: Import statements look good.The added imports are appropriate for the new functionality and type safety requirements.
71-73: Well-structured preferences type.The
GenerateVarsPreferencestype provides a clean interface for controlling theme generation options.
75-117: Clean implementation of conditional font variables.The function correctly implements the opt-in behavior for font variables while maintaining backward compatibility with the default value of
true. The separation of variables between light and dark modes is appropriate.
119-130: Smart optimization for tracking variables.The function efficiently avoids generating tracking variables when letter-spacing is zero, reducing unnecessary CSS output.
131-201: Well-implemented Tailwind V4 inline theme generator.The function correctly handles the conditional inclusion of font variables and properly generates the
@theme inlineblock for Tailwind V4.
292-320: Correct propagation of preferences parameter.The function properly passes the preferences through to all relevant sub-functions while maintaining backward compatibility.
322-336: Preferences parameter correctly integrated.The function properly accepts and forwards the preferences parameter to the Tailwind V3 config generator.
|
Hi luis, can you pull latest changes and resolve the conflict? I know it's been a while since I've looked at PRs. apologies! |
|
Yes I will pull the latest changes! |
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
components/editor/code-panel.tsx (1)
199-201: Debug background removed.The accidental red background is gone. Thanks for cleaning this up.
🧹 Nitpick comments (4)
components/editor/code-panel.tsx (4)
66-71: Memoize heavy code generation and gate v3 config computation.Avoid re-generating large strings on every render and skip Tailwind config when on v4.
- const preferences: GenerateVarsPreferences = { - includeFontVariables, - }; - - const code = generateThemeCode(themeEditorState, colorFormat, tailwindVersion, preferences); - const configCode = generateTailwindConfigFileCode(themeEditorState, preferences); + const preferences = useMemo<GenerateVarsPreferences>( + () => ({ includeFontVariables }), + [includeFontVariables] + ); + const code = useMemo( + () => generateThemeCode(themeEditorState, colorFormat, tailwindVersion, preferences), + [themeEditorState, colorFormat, tailwindVersion, preferences] + ); + const configCode = useMemo( + () => (tailwindVersion === "3" ? generateTailwindConfigFileCode(themeEditorState, preferences) : ""), + [themeEditorState, tailwindVersion, preferences] + );
235-236: Normalize icon size in Preferences button.Keeps icon sizing consistent with other buttons.
- <Settings /> + <Settings className="size-4" />
120-123: Coerce to boolean to avoidstring | falseinference.Prevents odd typings in conditions.
- const showRegistryCommand = useMemo(() => { - return preset && preset !== "default" && !hasUnsavedChanges(); - }, [preset, hasUnsavedChanges]); + const showRegistryCommand = useMemo( + () => Boolean(preset) && preset !== "default" && !hasUnsavedChanges(), + [preset, hasUnsavedChanges] + );
100-107: IncludeincludeFontVariablesin analytics payload.Useful for adoption tracking and correlating copied code with preferences.
const captureCopyEvent = (event: string) => { posthog.capture(event, { editorType: "theme", preset, colorFormat, tailwindVersion, + includeFontVariables, }); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
components/editor/code-panel.tsx(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/editor/code-panel.tsx (6)
types/editor.ts (1)
ThemeEditorState(18-27)hooks/use-dialog-actions.tsx (1)
useDialogActions(219-227)store/editor-store.ts (1)
useEditorStore(34-231)store/preferences-store.ts (1)
usePreferencesStore(29-73)utils/theme-style-generator.ts (3)
GenerateVarsPreferences(71-73)generateThemeCode(292-320)generateTailwindConfigFileCode(322-336)types/index.ts (1)
ColorFormat(53-53)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Vade Review
🔇 Additional comments (3)
components/editor/code-panel.tsx (3)
124-130: Auto-tab switch logic looks good.Prevents a dead tab when switching to Tailwind v4.
258-265: Controlled tabs with constants = ✅Improves readability and avoids magic strings.
210-216: Fix invalid Tailwind classes (z-99999,outline-hidden).
z-99999isn’t a default Tailwind class (use an arbitrary value) andoutline-hiddenis invalid (useoutline-none). These break layering/focus styles for Select/Popover.- <SelectTrigger className="bg-muted/50 w-fit gap-1 border-none outline-hidden focus:border-none focus:ring-transparent"> + <SelectTrigger className="bg-muted/50 w-fit gap-1 border-none outline-none focus:border-none focus:ring-transparent"> ... - <SelectContent className="z-99999"> + <SelectContent className="z-[9999]"> ... - <Select value={colorFormat} onValueChange={(value: ColorFormat) => setColorFormat(value)}> + <Select value={colorFormat} onValueChange={(value: ColorFormat) => setColorFormat(value)}> <SelectTrigger className="bg-muted/50 w-fit gap-1 border-none outline-hidden focus:border-none focus:ring-transparent"> - <SelectValue className="focus:ring-transparent" /> + <SelectValue className="focus:ring-transparent" /> </SelectTrigger> - <SelectContent className="z-99999"> + <SelectContent className="z-[9999]"> ... - <PopoverContent align="end" className="z-99999 w-[300px] space-y-2"> + <PopoverContent align="end" className="z-[9999] w-[300px] space-y-2">Also applies to: 219-229, 240-241
⛔ Skipped due to learnings
Learnt from: SameerJS6 PR: jnsahaj/tweakcn#208 File: components/editor/code-panel.tsx:188-188 Timestamp: 2025-08-03T11:11:04.843Z Learning: In the ResponsiveDialog implementation (revola.tsx), SelectContent components within dialog content use z-99999 to ensure proper layering above ResponsiveDialogOverlay which uses z-50. This high z-index is intentional for the responsive dialog system layering.Learnt from: SameerJS6 PR: jnsahaj/tweakcn#208 File: components/editor/code-panel.tsx:188-188 Timestamp: 2025-08-03T11:11:04.843Z Learning: In the ResponsiveDialog implementation (revola.tsx), SelectContent components within dialog content use z-99999 to ensure proper layering above ResponsiveDialogContent and ResponsiveDialogOverlay which use z-50. This high z-index is intentional for the responsive dialog system.
|
This support the optional export fonts option merging the latest changes from main |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
components/editor/code-panel.tsx (4)
66-71: Memoize code generation and gate config generation to v3Avoid regenerating large strings every render. Also skip Tailwind config code when on v4.
Apply:
- const preferences: GenerateVarsPreferences = { - includeFontVariables, - }; - - const code = generateThemeCode(themeEditorState, colorFormat, tailwindVersion, preferences); - const configCode = generateTailwindConfigFileCode(themeEditorState, preferences); + const preferences = useMemo<GenerateVarsPreferences>( + () => ({ includeFontVariables }), + [includeFontVariables] + ); + + const code = useMemo( + () => generateThemeCode(themeEditorState, colorFormat, tailwindVersion, preferences), + [themeEditorState, colorFormat, tailwindVersion, preferences] + ); + + const configCode = useMemo( + () => + tailwindVersion === "3" + ? generateTailwindConfigFileCode(themeEditorState, preferences) + : "", + [themeEditorState, tailwindVersion, preferences] + );
213-213: Non-standard Tailwind class “z-99999”Tailwind doesn’t ship
z-99999. Use arbitrary value syntax or a token you’ve defined.- <SelectContent className="z-99999"> + <SelectContent className="z-[99999]">- <PopoverContent align="end" className="z-99999 w-[300px] space-y-2"> + <PopoverContent align="end" className="z-[99999] w-[300px] space-y-2">Also applies to: 240-240
203-208: Centralize Tailwind-version → color-format coercionThe v3→hsl rule lives in the store, but the v4→oklch rule lives here. Co-locate both in
setTailwindVersionto keep a single source of truth and avoid drift.
260-260: Redundant defaultValue on controlled TabsYou already control
valuevia state; dropdefaultValue.- defaultValue={EXPORT_CODE_TABS.CSS_CODE}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/editor/code-panel.tsx(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/editor/code-panel.tsx (4)
types/editor.ts (1)
ThemeEditorState(18-27)store/preferences-store.ts (1)
usePreferencesStore(29-73)utils/theme-style-generator.ts (3)
GenerateVarsPreferences(71-73)generateThemeCode(292-320)generateTailwindConfigFileCode(322-336)types/index.ts (1)
ColorFormat(53-53)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Vade Review
🔇 Additional comments (2)
components/editor/code-panel.tsx (2)
120-123: Verify Zustand selector — subscribe to a boolean, not the methodSelecting
state.hasUnsavedChangeslikely returns the function reference (identity doesn't change), so the component won't re-render when the dirty state toggles.
- Preferred: subscribe to a boolean/counter slice. Example:
const hasUnsaved = useStore(s => s.isDirty);
const showRegistryCommand = preset && preset !== "default" && !hasUnsaved;- Or call the method inside the selector so the selector returns a primitive:
const hasUnsaved = useStore(s => s.hasUnsavedChanges());
const showRegistryCommand = preset && preset !== "default" && !hasUnsaved;- If you cannot change the selector, remove
useMemoand compute inline — but ensure the value used is a reactive primitive that actually changes.Could not verify the store file due to the earlier script failure; confirm whether
hasUnsavedChangesis exposed as a function reference or a boolean and update the selector accordingly.
210-221: Verifyoutline-hiddenutilityMultiple UI components (e.g. components/editor/code-panel.tsx:210–221 and many files under components/ui/) use
outline-hidden, but a repo search did not reveal atailwind.config.*or a.outline-hiddenCSS rule. Tailwind’s builtin isoutline-none; ifoutline-hiddenisn’t defined it’s a no-op — define it (plugin/theme.extend/addUtilities or global CSS) or replace usages withoutline-none.
Additions:
2025-07-28.17-40-41.mp4
These are the diffs in code for
:root,.darkand@theme inline.Same theme, but with vars cleaned up.
Summary by CodeRabbit
New Features
UI Improvements