Repository navigation
fix(core): floor SegmentedControlItem to a 44px touch target on coarse pointers - #6084
ManoharPaturi wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
github-actions
Bot
added
community
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewNo new or modified components detected. Bundle Size Summary
Accessibility AuditStatus: No accessibility violations detected. Visual RegressionStatus: No visual change across 420 compared shot(s). Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
…e pointers Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com>
3c610be to
08a3883
Compare
|
@cixzhang @kentonquatman this one has been sitting green since early september, is there anything holding it up that i can help with? happy to add tests or adjust the approach if the touch-target floor via the coarse-pointer media query isn't the shape you want. |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approval needs a maintainer decision first: please provide or identify the governing spec that adopts a 44px coarse-pointer touch target for SegmentedControl, and settle whether the intended contract is a 44px minimum height only or a full 44 by 44 target.
No current spec was found that settles this — the adopted accessibility baseline is WCAG 2.2 AA, whose target-size minimum is 24 by 24, and the component sizing records remain drafts. Until that contract is settled, the implementation cannot be approved as written. Separately, the doc, changeset, and code comment should not call 44px the WCAG AA minimum, and should not promise a 44px target in both dimensions while the rule floors only height and icon-only items remain 30/40/42px wide — please correct that wording, and add the inline floor and icon-only coverage if the settled contract is 44 by 44.
[Automated review]
There was a problem hiding this comment.
Approval needs a maintainer decision first: please provide or identify the governing spec that adopts a 44px coarse-pointer touch target for SegmentedControl, and settle whether the intended contract is a 44px minimum height only or a full 44 by 44 target.
No current spec was found that settles this — the adopted accessibility baseline is WCAG 2.2 AA, whose target-size minimum is 24 by 24, and the component sizing records remain drafts. Until that contract is settled, the implementation cannot be approved as written. Separately, the doc, changeset, and code comment should not call 44px the WCAG AA minimum, and should not promise a 44px target in both dimensions while the rule floors only height and icon-only items remain 30/40/42px wide — please correct that wording, and add the inline floor and icon-only coverage if the settled contract is 44 by 44.
[Automated review]
Fixes #6013
Summary
SegmentedControlItemtopped out at 32px (size="lg"=--size-element-lg− 4px), below the 44px minimum touch target of WCAG 2.2 AA "Target Size (Minimum)" / Apple HIG / Material — and sincelgis the ceiling, no prop combination could reach the floor.ButtonandToggleButtonalready honour the floor on phone surfaces; the segmented control's class was simply not in it.Change
A
@media (pointer: coarse)min-height: 44pxfloor on the item's base style — mirroringTouchDateField'smonthArrowfloor (min-heightscoped to coarse pointers,default: null). Every size is floored on touch; desktop density is unchanged (no unconditional rule), which is the part consumers cannot safely do themselves.Tests
New case in
SegmentedControl.test.tsxusing the repo'sinjectedRules()CSSOM pattern (mirroringInputClearButton): the item's classes have a coarse-pointer rule carryingmin-height: 44px, and no unconditional min-height exists. Verified the test fails on the unfloored component and passes with the fix.vitest run packages/core/src/SegmentedControl→ 44 passed; coretsc --noEmit, eslint,check:sync, andcheck:changesetsclean. Doc note on thesizeprop; changeset included.