Skip to content
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

fix(material/chips): remove button is too small #29351

Merged
merged 12 commits into from
Jul 17, 2024

Conversation

essjay05
Copy link
Contributor

@essjay05 essjay05 commented Jun 28, 2024

Fixes Angular Components Chip component where the touch target of the remove button of a chip is too small. Updates .mat-mdc-chip-remove to target more specific styles to override original style of padding: 8px so that the user has a larger touch target particularly on mobile devices.

Fixes b/286959517

Before screenshot
After screenshot

BREAKING CHANGE: updates chip remove button touch target to increase accessibility of the button especially on touch/mobile devices.

@essjay05 essjay05 force-pushed the increase-chip-remove-touch-target-size branch from 049bd97 to 347ecef Compare July 1, 2024 15:53
@angular-robot angular-robot bot added the detected: breaking change PR contains a commit with a breaking change label Jul 1, 2024
@essjay05 essjay05 force-pushed the increase-chip-remove-touch-target-size branch from 24e6edb to 1319eee Compare July 1, 2024 20:58
@essjay05 essjay05 changed the title fix(material/chip): remove button is too small fix(material/chips): remove button is too small Jul 1, 2024
@essjay05 essjay05 marked this pull request as ready for review July 1, 2024 21:33
@essjay05 essjay05 requested a review from a team as a code owner July 1, 2024 21:33
@essjay05 essjay05 requested review from crisbeto and amysorto and removed request for a team July 1, 2024 21:33
@mmalerba mmalerba added the dev-app preview When applied, previews of the dev-app are deployed to Firebase label Jul 1, 2024
Copy link

github-actions bot commented Jul 1, 2024

Deployed dev-app for 1442854 to: https://ng-dev-previews-comp--pr-angular-components-29351-dev-4ylvjidv.web.app

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

@essjay05 essjay05 requested a review from mmalerba July 2, 2024 22:49
@essjay05 essjay05 force-pushed the increase-chip-remove-touch-target-size branch 4 times, most recently from 3c5e924 to 97aa7b2 Compare July 9, 2024 15:52
@essjay05
Copy link
Contributor Author

essjay05 commented Jul 9, 2024

As recommended, tested on Mac Chrome and Firefox against React, Vue, W3, and dialog native html. The issue is reproducible in each of these when using a Mac OS.

Examples tested:
-React dialog demo
-Vue dialog demo
-W3.org dialog demo
-Native html dialog demo

@essjay05 essjay05 force-pushed the increase-chip-remove-touch-target-size branch from 97aa7b2 to 37d6220 Compare July 10, 2024 23:22
@essjay05 essjay05 requested a review from mmalerba July 10, 2024 23:25
src/material/chips/chip.scss Show resolved Hide resolved
src/material/chips/chip.scss Outdated Show resolved Hide resolved
src/material/chips/chip.scss Outdated Show resolved Hide resolved
@essjay05 essjay05 requested a review from mmalerba July 12, 2024 20:16
@essjay05 essjay05 force-pushed the increase-chip-remove-touch-target-size branch 2 times, most recently from 23e354f to 9c4960c Compare July 15, 2024 17:20
Fixes Angular Components Chip component where the touch target of the
remove button of a chip is too small. Updates .mat-mdc-chip-remove to
target more specific styles to override original style of padding: 8px
so that the user has a larger touch target particularly on mobile
devices.

Fixes b/286959517
Fixes lint errors from previous commit for Angular Components
Chip component where the remove button touch target is too small,
particularly for touch/mobile devices. Increases padding on all
sides.

Fixes b/286959517
Fixes issue with Angular Component Chip's touch target size by
increasing the padding on chips with a trailing icon that has
an action.

Fixes b/286959517

BREAKING CHANGE:  updates chip remove button touch target to increase accessibility of the button especially on touch/mobile devices.
Updates fix to Autocomplete Component Chips component where the
touch target size is too small and fails minimum accessibility
size of 48x48 px. Addresses nit fixes from PR review.

Fixes b/286959517

BREAKING CHANGE: updates padding size of remove touch target to
satisfy minimum 48x48 accessibility size.
Fixes Angular Components Chip component where the touch target of the
remove button of a chip is too small. Updates .mat-mdc-chip-remove to
target more specific styles to override original style of padding: 8px
so that the user has a larger touch target particularly on mobile
devices.

Fixes b/286959517
Fixes lint errors from previous commit for Angular Components
Chip component where the remove button touch target is too small,
particularly for touch/mobile devices. Increases padding on all
sides.

Fixes b/286959517
Fixes issue with Angular Component Chip's touch target size by
increasing the padding on chips with a trailing icon that has
an action.

Fixes b/286959517

BREAKING CHANGE:  updates chip remove button touch target to increase accessibility of the button especially on touch/mobile devices.
…essible

Updates Angular Components Chips component by increasing its
.mat-mdc-chip-remove::after styles to increase the size of the
touch target to make it more accessible/clickable.

Fixes b/286959517
Updates fix for Angular Components Chips component where there
were lint formatting errors.
Updates previous Angular Components Chip component fix and makes it
adaptable by swapping the hard-coded 18px offset to equal the
variable for -icon-size.
Updates previous Angular Component Chip component fix which
added padding to the ::after pseudo-element and attempted to
calculate centering. Changes calculation based on using
 with padding values countered
with margin negative values to center the touch target.
Updates Angular Component Chips component fix to change ::after
background property changes to be background-color specifically to
avoid overriding background-clip styles. Suppresses lint warning
material/no-prefixes for background-clip since it's majority
compatible with browsers.

Fixes b/286959517
@essjay05 essjay05 force-pushed the increase-chip-remove-touch-target-size branch from 9c4960c to 1442854 Compare July 16, 2024 17:49
@mmalerba
Copy link
Contributor

Looks good! I notice that the chip content seems slightly off center, but that appears to be an issue at HEAD, likely a regression introduced by #29357 and not something blocking for this PR

@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker merge: fix commit message When the PR is merged, rewrites/fixups of the commit messages are needed target: patch This PR is targeted for the next patch release labels Jul 17, 2024
@crisbeto crisbeto merged commit d6aed80 into angular:main Jul 17, 2024
25 of 28 checks passed
crisbeto pushed a commit that referenced this pull request Jul 17, 2024
* fix(material/chip): remove button is too small

Fixes Angular Components Chip component where the touch target of the
remove button of a chip is too small. Updates .mat-mdc-chip-remove to
target more specific styles to override original style of padding: 8px
so that the user has a larger touch target particularly on mobile
devices.

Fixes b/286959517

* refactor(material/chips): remove button is too small

Fixes lint errors from previous commit for Angular Components
Chip component where the remove button touch target is too small,
particularly for touch/mobile devices. Increases padding on all
sides.

Fixes b/286959517

* fix(material/chips): increase chip remove button touch target size

Fixes issue with Angular Component Chip's touch target size by
increasing the padding on chips with a trailing icon that has
an action.

Fixes b/286959517

BREAKING CHANGE:  updates chip remove button touch target to increase accessibility of the button especially on touch/mobile devices.

* refactor(material/chips): chip remove touch target size is insufficient

Updates fix to Autocomplete Component Chips component where the
touch target size is too small and fails minimum accessibility
size of 48x48 px. Addresses nit fixes from PR review.

Fixes b/286959517

BREAKING CHANGE: updates padding size of remove touch target to
satisfy minimum 48x48 accessibility size.

* fix(material/chip): remove button is too small

Fixes Angular Components Chip component where the touch target of the
remove button of a chip is too small. Updates .mat-mdc-chip-remove to
target more specific styles to override original style of padding: 8px
so that the user has a larger touch target particularly on mobile
devices.

Fixes b/286959517

* refactor(material/chips): remove button is too small

Fixes lint errors from previous commit for Angular Components
Chip component where the remove button touch target is too small,
particularly for touch/mobile devices. Increases padding on all
sides.

Fixes b/286959517

* fix(material/chips): increase chip remove button touch target size

Fixes issue with Angular Component Chip's touch target size by
increasing the padding on chips with a trailing icon that has
an action.

Fixes b/286959517

BREAKING CHANGE:  updates chip remove button touch target to increase accessibility of the button especially on touch/mobile devices.

* fix(material/chips): chips remove touch target is too small to be accessible

Updates Angular Components Chips component by increasing its
.mat-mdc-chip-remove::after styles to increase the size of the
touch target to make it more accessible/clickable.

Fixes b/286959517

* refactor(material/chips): fix lint errors

Updates fix for Angular Components Chips component where there
were lint formatting errors.

* refactor(material/chips): replace px with _trailing-icon-size variable

Updates previous Angular Components Chip component fix and makes it
adaptable by swapping the hard-coded 18px offset to equal the
variable for -icon-size.

* refactor(material/chips): updates touch target centering method

Updates previous Angular Component Chip component fix which
added padding to the ::after pseudo-element and attempted to
calculate centering. Changes calculation based on using
 with padding values countered
with margin negative values to center the touch target.

* refactor(material/chips): updates ::after styles and fix lint errors

Updates Angular Component Chips component fix to change ::after
background property changes to be background-color specifically to
avoid overriding background-clip styles. Suppresses lint warning
material/no-prefixes for background-clip since it's majority
compatible with browsers.

Fixes b/286959517

(cherry picked from commit d6aed80)
andrewseguin added a commit to andrewseguin/components that referenced this pull request Jul 17, 2024
andrewseguin added a commit that referenced this pull request Jul 17, 2024
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Aug 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker detected: breaking change PR contains a commit with a breaking change dev-app preview When applied, previews of the dev-app are deployed to Firebase merge: fix commit message When the PR is merged, rewrites/fixups of the commit messages are needed target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants