Skip to content
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

Feature: Use any lucide icon for the add node button #64

Merged
merged 13 commits into from
Oct 4, 2024

Conversation

buckhalt
Copy link
Member

@buckhalt buckhalt commented Sep 24, 2024

Implements ability to use any lucide icon for the add node button using dynamic imports

Copy link

vercel bot commented Sep 24, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 4, 2024 10:08am
studio-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 4, 2024 10:08am

components/interview/ActionButton.tsx Outdated Show resolved Hide resolved
components/interview/icons/LucideAddNode.tsx Outdated Show resolved Hide resolved
components/interview/icons/LucideAddNode.tsx Outdated Show resolved Hide resolved
schemas/protocol/codebook/entities.ts Show resolved Hide resolved
schemas/protocol/codebook/entities.ts Outdated Show resolved Hide resolved
@jthrilly
Copy link
Member

jthrilly commented Oct 4, 2024

Okay, made some fairly substantial changes to this:

  • Created a storybook for this component that demonstrates color and icon props. This should always be done for UI component dev.
  • Looked at the implementation details, and worked around quite a nasty bug/design flaw in the lucide dynamic icon functionality: Dynamic Import makes NextJS development server very slow lucide-icons/lucide#1576 (comment)
  • Looked again at the theming behaviour. I just wasn't happy with not having hot module reload for themes, and having to put stylesheets in public in order to get this to work on storybook. Ended up implementing a fairly convoluted workaround/hack that allows stylesheets to be loaded/unloaded with a component, and then implemented a component that is conditionally rendered in a storybook decorator. This allows for the behaviour we want. Since we don't plan to use dynamic theme switching in the main app now, we can just import the relevant stylesheets normally there.

@jthrilly jthrilly merged commit 08e41f3 into main Oct 4, 2024
6 checks passed
@jthrilly jthrilly deleted the feature/lucide-add-node branch October 4, 2024 10:10
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.

2 participants