-
Notifications
You must be signed in to change notification settings - Fork 377
Style: Make components themeable #5908
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 10/05/2025, 07:52:27 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results❌ Some tests failed ⏰ Completed at: 10/05/2025, 08:05:22 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
src/services/colorPaletteService.ts
Outdated
WIDGET_TEXT_COLOR: 'node-component-widget-input' | ||
// /* TOGGLE | ||
} as const satisfies Partial<Record<keyof Colors['litegraph_base'], string>> | ||
/*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: we can take away this comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but it's useful to see which properties haven't yet had equivalent tokens defined.
|
||
## Icon Usage in Storybook | ||
|
||
In this project, the `<i-lucide:... />` syntax from unplugin-icons is not supported in Storybook. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Claude and Codex thank you.
## Icon Usage in Storybook | ||
|
||
In this project, the `<i-lucide:... />` syntax from unplugin-icons is not supported in Storybook. | ||
In this project, the `<i class="icon-[lucide--folder]" />` syntax from unplugin-icons is not supported in Storybook. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
non-blocker: This should go in the root CLAUDE.md as well. Will prevent many bad generations.
root: { | ||
class: | ||
'!w-7 !h-7 !border-none !outline-none !p-2 !m-1.5 bg-gray-500 dark-theme:bg-neutral-700 text-white' | ||
'!w-7 !h-7 !border-none !outline-none !p-2 !m-1.5 bg-dialog-surface text-white' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
non-blocker: this is where we'd have to start removing !importants if we wanted to fix the dialogs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yessssss
Summary
Replace color/dark-color pairs in components with design tokens to allow for easy overriding.
Also standardizes the icon pattern to simplify the tailwind config.
Changes
Review Focus
Still have quite a few places from which to remove
dark-theme
, but this at least gets the theming much closer.Need to decide if I want to keep going in here or cut this and do the rest in a subsequent PR.
Screenshots (if applicable)
┆Issue is synchronized with this Notion page by Unito