-
Notifications
You must be signed in to change notification settings - Fork 53
Increase the touch target size of SubNav.SubMenu toggle button #1017
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
Conversation
🦋 Changeset detectedLatest commit: d4acd41 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
🟢 No design token changes found |
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.
Pull Request Overview
Increases the touch target size of the SubNav.SubMenu
toggle button from 16×16 to 24×24 without altering its visual position by adding height, inline padding, and compensating negative margin.
- Adjusted
.SubNav__sub-menu-toggle
CSS to enforce a 24px height and expand clickable area. - Provided a changeset entry to document the patch release.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
packages/react/src/SubNav/SubNav.module.css | Added height , padding-inline-end , and margin-inline-end to expand the toggle’s touch target |
.changeset/tiny-bears-lick.md | Introduced a changeset recording the UI adjustment |
Comments suppressed due to low confidence (1)
packages/react/src/SubNav/SubNav.module.css:633
- It looks like there isn’t an updated visual snapshot or test verifying the new 24×24 touch target; consider adding or updating a snapshot to catch regressions for this change.
height: var(--base-size-24);
d4acd41
to
14f5f65
Compare
🟢 No visual differences foundOur visual comparison tests did not find any differences in the UI. |
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.
LGTM!
Summary
Increased the touch target size of
SubNav.SubMenu
toggle button to 24px × 24px.The button was previously only 16px × 16px, and rather than add 4px to the left and right I've instead added 8px to the right (along with -8px of margin-right) so as to not cause any visual changes. You can see the touch target size change in the screenshots below.
What should reviewers focus on?
Supporting resources (related issues, external links, etc):
Contributor checklist:
update snapshots
label to the PR)Reviewer checklist:
Screenshots: