checkbox variant - outline #1067
Conversation
mackbrowne
force-pushed
the
feature/outline-checkbox
branch
from
6158b3f to
ad595f2
Compare
There was a problem hiding this comment.
Hi @mackbrowne
Great feature. Thanks for the PR.
See my comment, I found two issues.
src/components/checkbox/index.tsx
Outdated
| > | ||
| <Animated.Image | ||
| style={[ | ||
| this.styles.selectedIcon, | ||
| color && {tintColor: iconColor}, | ||
| color && {tintColor: outline ? this.getColor() : DEFAULT_ICON_COLOR}, |
There was a problem hiding this comment.
There's an issue here.
You ignore the iconColor prop.
If a checkbox starts disabled and then become enabled in runtime.
The checkmark icon color will stay grey
Also, with when passing the outline flag, iconColor is being ignored.
I suggest moving the logic of the icon color to a separate function (getIconColor) and apply in runtime.
setting it in createStyles will not take effect in runtime..
There was a problem hiding this comment.
@ethanshar thanks for the quick feedback! I'll add this soon.
I also noticed a bug with 'outline - no color - disabled` seen in the screenshot above which I think is caused by the runtime scenario you explained. I'll do another round of cleaning it up today and get back to you.
There was a problem hiding this comment.
@ethanshar this should be fixed now with the new commit!
I also took the liberty of creating DEFAULT_BORDER_WIDTH and DEFAULT_BORDER_RADIUS constants and included them with the rest of the constants for this component. I hope this is ok!
There was a problem hiding this comment.
@ethanshar what is the process here? Should I resolve this or leave it for you to review then close?
There was a problem hiding this comment.
Hi,
Sorry for the late response.
There's still small issue.
See my comment.
mackbrowne
force-pushed
the
feature/outline-checkbox
branch
from
3ddd9c3 to
a8b2c88
Compare
…EFAULT_BORDER_WIDTH
mackbrowne
force-pushed
the
feature/outline-checkbox
branch
from
a8b2c88 to
b9f4332
Compare
|
Updated Screenshots
|
Description
Alternative style for checkbox that wasn't possible with basic styling and theming alone.
Changelog
Added optional boolean outline prop. The checkbox acts normal when false and by default.