Skip to content

fix(material/slide-toggle): increase slide toggle ripple touch target #31469

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

adolgachev
Copy link
Contributor

No description provided.

@adolgachev adolgachev marked this pull request as ready for review July 1, 2025 18:01
@adolgachev adolgachev requested a review from a team as a code owner July 1, 2025 18:01
@adolgachev adolgachev requested review from crisbeto, wagnermaciel and andrewseguin and removed request for a team, crisbeto and wagnermaciel July 1, 2025 18:01
@adolgachev adolgachev added Accessibility This issue is related to accessibility (a11y) target: minor This PR is targeted for the next minor release dev-app preview When applied, previews of the dev-app are deployed to Firebase action: review The PR is still awaiting reviews from at least one requested reviewer action: global presubmit The PR is in need of a google3 global presubmit requires: TGP This PR requires a passing TGP before merging is allowed labels Jul 2, 2025
Copy link

github-actions bot commented Jul 2, 2025

Deployed dev-app for 01c65da to: https://ng-dev-previews-comp--pr-angular-components-31469-dev-xl6bs3gm.web.app

Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt.

-1: 36px,
-2: 32px,
-3: 28px,
0: 48px,
Copy link
Member

@crisbeto crisbeto Jul 2, 2025

Choose a reason for hiding this comment

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

I don't think that this is the right way to solve the issue, it just makes the ripple larger and ripples are meant to be decorative. The way we handle the touch target in other components is to have a transparent div that is always at least 48x48 and isn't affected by density. See .mat-mdc-radio-touch-target as an example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was suggested by @andrewseguin actually instead of #31178.

I'll take a look at mat-mdc-radio-touch-target.

Another issue with increasing the ripple is it can add a scrollbar when the toggle is at the bottom, breaking quite a few screenshot tests.

Copy link
Member

Choose a reason for hiding this comment

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

I think that #31178 is the proper way to do it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That does actually look like a simpler and better solution that either of the others. Started at #31486.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accessibility This issue is related to accessibility (a11y) action: global presubmit The PR is in need of a google3 global presubmit action: review The PR is still awaiting reviews from at least one requested reviewer area: material/slide-toggle dev-app preview When applied, previews of the dev-app are deployed to Firebase requires: TGP This PR requires a passing TGP before merging is allowed target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants