-
Notifications
You must be signed in to change notification settings - Fork 405
RI-7046: replace EuiCheckbox #4576
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
RI-7046: replace EuiCheckbox #4576
Conversation
redisinsight/ui/src/components/base/forms/checkbox/Checkbox.tsx
Outdated
Show resolved
Hide resolved
cfef56e
to
612ce31
Compare
…vide a built-in prop to control label size
612ce31
to
14798bd
Compare
import RedisAIDark from 'uiSrc/assets/img/modules/RedisAIDark.svg?react' | ||
import { Icon, IconProps } from 'uiSrc/components/base/icons' | ||
|
||
export const RedisAIDarkIcon = (props: IconProps) => ( | ||
<Icon icon={RedisAIDark} {...props} /> |
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.
it's generally more maintainable to have a single reusable Icon component instead of {iconsCount} wrapper components
my suggestion would obviously require reverting lots of changes and making the PR smaller
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.
It is a single reusble component. The Icon component is the component, that wraps SVG's
If you suggest a component like EuiIcon to which you pass a string which indicates what icon do you want, then I have to disagree.
This approach requires the "MAIN ICON" component to import ALL svgs and map them to strings which one can then use. Which means that importing even a single icon will import them all in any bundle chunk
No description provided.