-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: example urls with icons #9368
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
Conversation
|
Build successful! 🎉 |
| import {TabLink} from './FileTabs'; | ||
| import {useLocale} from 'react-aria'; | ||
| // eslint-disable-next-line | ||
| import icons from '/packages/@react-spectrum/s2/s2wf-icons/*.svg'; |
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.
The problem with this is that it breaks the code splitting, so now every icon will be loaded on every page even if the component doesn't have an icon picker...
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.
Ah, hmm is there a way to send it as a rsc?
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.
Otherwise I guess I could send the svg directly
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.
You could maybe load it dynamically if you export the icon map from the icon picker file?
const LazyIcon = lazy(() => import('./IconPicker')
.then(({icons}) => ({default: ({icon}) => createElement(icons[icon])}))essentially this creates a LazyIcon component that accepts an icon prop and gets that icon from the map.
<LazyIcon icon="whatever" />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.
sure yeah, could do that
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.
Or even better put the LazyIcon component in the IconPicker file and lazily load that. Idk what I was thinking
|
Build successful! 🎉 |
Closes
Go to a doc page like RSP's ActionButton, add an icon, get the share url for the example and paste it into a new window. It should render with no errors and the icon should be displayed.
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: