-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[UI v2] feat: Adds EmptyState for Task Run Concurrency Limits #16203
[UI v2] feat: Adds EmptyState for Task Run Concurrency Limits #16203
Conversation
}; | ||
export const TaskRunConcurrencyLimitEmptyState = ({ onClick }: Props) => ( | ||
<EmptyState> | ||
<EmptyStateIcon id="CircleArrowOutUpRight" /> |
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.
Random thought: does defining the icon by ID like this affect tree shaking for react-lucide
?
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.
I don't think so: https://lucide.dev/guide/packages/lucide-react#one-generic-icon-component
I think the next followup PR, we can create a list of Icons only available to the ui using its direct imports.
And then we can create a Record of ui-v2's Icons and export it that way
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.
Gotcha, that sounds like a good way to abstract icon stuff and make it easy to introduce icons from other sources.
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.
Addressing in this PR: #16208
988b443
to
c85b81b
Compare
Checklist
<link to issue>
"mint.json
.Related to #15512