Touchables 48x48 hitslop support #3857
Conversation
added better hitslop support for slider thumb.
lidord-wix
requested review from
adids1221
and removed request for
lidord-wix
| } | ||
|
|
||
| state: Record<'size', undefined | number> = { | ||
| state: ButtonState = { |
There was a problem hiding this comment.
You switched to using ButtonState type (which is great!), but measuredSize and borderRadius aren't initialized in the state object. They're set later in onLayout, but it's better to initialize them upfront for clarity.
There was a problem hiding this comment.
so should I just initialize them both to be undefined here?
There was a problem hiding this comment.
I think it's better for readability, WDYT ?
|
|
||
| needsHitslopMeasurement() { | ||
| const {avoidMinWidth, avoidInnerPadding, round} = this.props; | ||
| return this.isLink || (this.isIconButton && !round) || avoidMinWidth || avoidInnerPadding; |
There was a problem hiding this comment.
Why this (this.isIconButton && !round) checking is related here ?
There was a problem hiding this comment.
for round buttons in general we have a separate measurement happening already in the code, and icon buttons have an issue where the button size depends on the icon size (there's a separate check in the code where if an icon button is non-round no padding is applied and no minWidth is set), so we have to measure its dimensions in this case.
adids1221
left a comment
adids1221
left a comment
There was a problem hiding this comment.
Nice work, left few small comments.
|
#rebuild |
Description
Added better hitslop calculation for buttons to handle edge cases i.e. link/icon buttons.
Added better hitslop calculation for slider thumb.
all touchable component should have a 48x48 hitslop now.
Changelog
Button and Slider Thumb Fixed hit target to be at least of 48x48.
Additional info
MADS-4840