-
Notifications
You must be signed in to change notification settings - Fork 34
feat(#2361): increase clickable area for radio and checkbox #3099
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
base: alpha
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -98,4 +98,67 @@ describe("Radio", () => { | |
| expect(selectedValue.element().textContent).toBe("apple"); | ||
| }); | ||
| }); | ||
|
|
||
| it("should have a 44px x 44px touch target area", async () => { | ||
| const result = render( | ||
| <GoabRadioGroup name="test" value=""> | ||
| <GoabRadioItem name="test" value="option1" label="Option 1" /> | ||
| </GoabRadioGroup> | ||
| ); | ||
|
|
||
| const radioInput = result.getByTestId("radio-option-option1"); | ||
| await vi.waitFor(() => { | ||
| expect(radioInput.element()).toBeTruthy(); | ||
| }); | ||
|
|
||
| // Get the parent label element and find the .icon element | ||
| const label = radioInput.element().closest("label"); | ||
| expect(label).toBeTruthy(); | ||
|
|
||
| const icon = label?.querySelector(".icon") as HTMLElement; | ||
| expect(icon).toBeTruthy(); | ||
|
|
||
| // Get computed styles for the ::before pseudo-element (touch target) | ||
| const beforeStyles = window.getComputedStyle(icon, "::before"); | ||
|
|
||
| // Verify the touch target dimensions | ||
| expect(beforeStyles.width).toBe("44px"); | ||
| expect(beforeStyles.height).toBe("44px"); | ||
| expect(beforeStyles.position).toBe("absolute"); | ||
|
|
||
| // Verify the icon itself has position: relative for proper positioning context | ||
| const iconStyles = window.getComputedStyle(icon); | ||
| expect(iconStyles.position).toBe("relative"); | ||
|
|
||
| // Verify the actual visual size of the icon (24px) vs touch target (44px) | ||
| const iconRect = icon.getBoundingClientRect(); | ||
| expect(iconRect.width).toBe(24); // Visual icon is 24px | ||
| expect(iconRect.height).toBe(24); // Visual icon is 24px | ||
|
|
||
| // Verify the transform is applied correctly for centering | ||
| // CSS: transform: translate(-50%, -50%) converts to matrix(a, b, c, d, tx, ty) | ||
| // Matrix breakdown: | ||
| // - (1, 0, 0, 1) = identity matrix (no scaling/rotation) | ||
| // - (-22, -22) = translate by -22px in X and Y directions | ||
| // Math: 50% of 44px = 22px, so translate(-50%, -50%) = translate(-22px, -22px) | ||
| // This centers the 44px touch target on the 24px icon | ||
| expect(beforeStyles.transform).toBe("matrix(1, 0, 0, 1, -22, -22)"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment as the one I made for the Checkbox tests |
||
|
|
||
| // Check ::after pseudo-element (should not interfere with touch target) | ||
| const afterStyles = window.getComputedStyle(icon, "::after"); | ||
| // ::after should not have conflicting dimensions or positioning | ||
| expect(afterStyles.position).not.toBe("absolute"); | ||
|
|
||
| // Final verification: Check that all styles are applied and rendered | ||
| // After the page is fully loaded and all CSS is computed | ||
| await vi.waitFor(() => { | ||
| const finalIconStyles = window.getComputedStyle(icon); | ||
| const finalBeforeStyles = window.getComputedStyle(icon, "::before"); | ||
|
|
||
| // Verify final computed styles match expectations | ||
| expect(finalIconStyles.position).toBe("relative"); | ||
| expect(finalBeforeStyles.width).toBe("44px"); | ||
| expect(finalBeforeStyles.height).toBe("44px"); | ||
| }); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -165,12 +165,12 @@ | |
| const checkboxEl = (_rootEl.getRootNode() as ShadowRoot)?.host as HTMLElement; | ||
| const fromCheckboxList = checkboxEl?.closest("goa-checkbox-list") !== null; | ||
|
|
||
| relay<FormFieldMountRelayDetail>( | ||
| _rootEl, | ||
| FormFieldMountMsg, | ||
| { name, el: _rootEl }, | ||
| { bubbles: !fromCheckboxList, timeout: 10 }, | ||
| ); | ||
| relay<FormFieldMountRelayDetail>( | ||
| _rootEl, | ||
| FormFieldMountMsg, | ||
| { name, el: _rootEl }, | ||
| { bubbles: !fromCheckboxList, timeout: 10 }, | ||
| ); | ||
| } | ||
|
|
||
| function onChange(e: Event) { | ||
|
|
@@ -387,6 +387,7 @@ max-width: ${maxwidth}; | |
|
|
||
| /* Container */ | ||
| .container { | ||
| position: relative; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only with Safari, when I am having a checkbox, and when I click on it, there is a light "shift" 22px to 24px around the checkbox, like the below video. Safari I am testing both 17.4.1 and 18.4 (browserstack) checkbox-safari.mov |
||
| box-sizing: border-box; | ||
| border: var(--goa-checkbox-border); | ||
| border-radius: var(--goa-checkbox-border-radius); | ||
|
|
@@ -398,6 +399,17 @@ max-width: ${maxwidth}; | |
| justify-content: center; | ||
| flex: 0 0 auto; /* prevent squishing of checkbox */ | ||
| } | ||
|
|
||
| .container::before { | ||
vanessatran-ddi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| content: ''; | ||
| position: absolute; | ||
| width: 44px; | ||
| height: 44px; | ||
| top: 50%; | ||
| left: 50%; | ||
| transform: translate(-50%, -50%); | ||
| } | ||
|
|
||
| .container:hover { | ||
| border: var(--goa-checkbox-border-hover); | ||
| } | ||
|
|
||
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.
This is extremely specific and I think very brittle. After the tests above, I'm not sure this is adding anything useful, and would make the test far more brittle than I would like.