Skip to content

Carousel - page control hit slop fix a11y #3763

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

Merged
merged 5 commits into from
Jun 19, 2025

Conversation

adids1221
Copy link
Contributor

Description

Carousel - pageControl set hitSlop as part of a11y fix.
Moved getAccessibleHitSlop function into StyleUtils to make reuse instead of duplications.

Changelog

Carousel - pageControl set hitSlop as part of a11y fix.

Additional info

MADS-4757

Copy link
Collaborator

@ethanshar ethanshar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Asked a question..
it's possible they detect a small hit slop and automatically report it, so I hope this would be good enough
Anyway approved

@@ -201,6 +202,10 @@ class PageControl extends PureComponent<PageControlProps, State> {
getColorStyle(index === currentPage, color, inactiveColor),
getSizeStyle(size, index, currentPage, enlargeActive)
]}
hitSlop={{
top: hitSlopValue,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you on purpose didn't assign the left/right hit slop?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, Alexey approved it also.
When assigning left/right they are overlapping each other, we don't want to change the UX specs of the PageControl for now.

@adids1221 adids1221 merged commit aae880e into master Jun 19, 2025
1 check passed
@adids1221 adids1221 deleted the fix/PageControl_hitSlop_a11y branch June 19, 2025 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants