-
Notifications
You must be signed in to change notification settings - Fork 725
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
feat(checkbox/radioButton): ensure minimum 48x48 hit target for better accessibility #3518
base: master
Are you sure you want to change the base?
feat(checkbox/radioButton): ensure minimum 48x48 hit target for better accessibility #3518
Conversation
…r accessibility Co-Authored-By: nitzany@wix.com <nitzany@wix.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Co-Authored-By: nitzany@wix.com <nitzany@wix.com>
Co-Authored-By: nitzany@wix.com <nitzany@wix.com>
… functions Co-Authored-By: nitzany@wix.com <nitzany@wix.com>
Co-Authored-By: nitzany@wix.com <nitzany@wix.com>
Co-Authored-By: nitzany@wix.com <nitzany@wix.com>
Co-Authored-By: nitzany@wix.com <nitzany@wix.com>
Co-Authored-By: nitzany@wix.com <nitzany@wix.com>
Co-Authored-By: nitzany@wix.com <nitzany@wix.com>
Please update the pr description according to our template. |
Thank you devin. I'll take it from here. Don't respond to anything here anymore. |
@@ -257,6 +257,16 @@ class RadioButton extends PureComponent<Props, RadioButtonState> { | |||
); | |||
} | |||
|
|||
getAccessibleHitSlop() { | |||
const {size = DEFAULT_SIZE} = this.props; | |||
const verticalPadding = Math.max(0, (48 - size) / 2); |
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.
Why only vertical?
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.
Horizontally the CheckBox
takes as much room as it can so it depends on the parent
src/components/checkbox/index.tsx
Outdated
@@ -252,8 +252,16 @@ class Checkbox extends Component<CheckboxProps, CheckboxState> { | |||
}; | |||
}; | |||
|
|||
getAccessibleHitSlop() { | |||
const {size = DEFAULT_SIZE} = this.props; | |||
const hitTargetPadding = Math.max(0, (48 - size) / 2); |
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.
return Math.max(0, (48 - size) / 2);
src/components/checkbox/index.tsx
Outdated
@@ -252,8 +252,16 @@ class Checkbox extends Component<CheckboxProps, CheckboxState> { | |||
}; | |||
}; | |||
|
|||
getAccessibleHitSlop() { |
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 would make it a pure function that gets the size and returns the hitslop
Description
This PR improves the accessibility of the Checkbox and RadioButton components by ensuring a minimum hit target size of 48x48 pixels, following WCAG guidelines. The changes maintain the visual appearance while making the components more accessible for touch interactions.
Changelog
Additional info
Ticket: MADS-4591