-
Notifications
You must be signed in to change notification settings - Fork 6
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
Implement Choice Tile #91
Conversation
Deploy preview for helix-react ready! Built with commit 2c5553d |
...rest | ||
}) => { | ||
return ( | ||
<label className={classNames({ hxChoice: true, className })}> |
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 blocking: a potential shorter way to say the same thing:
<label className={classNames({ hxChoice: true, className })}> | |
<label className={classNames( 'hxChoice', className)}> |
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 think this alternative would add a class name of undefined when there is no className
, right?
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.
only if it was written like this: classNames={`hxChoice ${className}`}
but className()
does not include undefined values.
from the docs: https://github.com/JedWatson/classnames#readme
// other falsy values are just ignored
classNames(null, false, 'bar', undefined, 0, 1, { baz: null }, ''); // => 'bar 1'
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.
Nice work!
Dev LGTM
https://helixdesignsystem.github.io/helix-ui/components/choice-tile/