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): fix click target when stacked #27129

Merged
merged 1 commit into from May 31, 2023

Conversation

zarend
Copy link
Contributor

@zarend zarend commented May 20, 2023

Ensure that the click target of stacked chips is the entire width of the chip. Fix issue where applying .mat-mdc-chip-set-stacked causes chips to respond to clicks when clicking on the text; did not respond when clicking inside the chip but not over the text (#26590).

Ensure the clickable elemen tof the chip takes up the whole visual area of the chip by overriding styles from MDC library. Angular Material supports vertically-stacked chips, which MDC does not.

Fix #26590

@zarend zarend added Accessibility This issue is related to accessibility (a11y) target: patch This PR is targeted for the next patch release area: material/chips dev-app preview When applied, previews of the dev-app are deployed to Firebase labels May 20, 2023
@github-actions
Copy link

github-actions bot commented May 20, 2023

Deployed dev-app for 674eea2 to: https://ng-dev-previews-comp--pr-angular-components-27129-ixblu3cc.web.app

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

@zarend zarend added the action: review The PR is still awaiting reviews from at least one requested reviewer label May 22, 2023
@zarend zarend marked this pull request as ready for review May 22, 2023 15:42
@zarend zarend force-pushed the stack-chips branch 2 times, most recently from d1382bd to 92e38c1 Compare May 30, 2023 20:09
Ensure that the click target of stacked chips is the entire width of the
chip. Fix issue where applying .mat-mdc-chip-set-stacked causes chips to
respond to clicks when clicking on the text; did not respond when
clicking inside the chip but not over the text (angular#26590).

Ensure the clickable elemen tof the chip takes up the whole visual area
of the chip by overriding styles from MDC library. Angular Material
supports vertically-stacked chips, which MDC does not.

Fix angular#26590
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

LGTM

@zarend
Copy link
Contributor Author

zarend commented May 30, 2023

@crisbeto , when testing with trailing icons, I noticed a problem that doesn't look like a regression caused by this PR. Was wondering if you could help me understand how interactive vs decorative trailing icons are intended to work.

When I click on the trailing icon, the chip ripples but it doesn't trigger the click handler. This happens in both horizontal and stacked, so I don't think it's a regression. I would expect clicking a trailing icon to do the same thing as clicking an icon in the front of the chip.

Is MatChipTrailingIcon supposed to be non-interactive by default?

image

@zarend zarend added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels May 30, 2023
@crisbeto
Copy link
Member

crisbeto commented May 30, 2023

Yeah trailing icons are non-interactive by default. Only the chip remove icon is interactive. The screenshot looks correct to me since only the primary action is interactive in your example.

@angular-robot angular-robot bot merged commit 2c07867 into angular:main May 31, 2023
24 checks passed
angular-robot bot pushed a commit that referenced this pull request May 31, 2023
Ensure that the click target of stacked chips is the entire width of the
chip. Fix issue where applying .mat-mdc-chip-set-stacked causes chips to
respond to clicks when clicking on the text; did not respond when
clicking inside the chip but not over the text (#26590).

Ensure the clickable elemen tof the chip takes up the whole visual area
of the chip by overriding styles from MDC library. Angular Material
supports vertically-stacked chips, which MDC does not.

Fix #26590

(cherry picked from commit 2c07867)
@zarend zarend deleted the stack-chips branch May 31, 2023 20:41
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Jun 7, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@angular/cdk](https://github.com/angular/components) | dependencies | patch | [`16.0.1` -> `16.0.3`](https://renovatebot.com/diffs/npm/@angular%2fcdk/16.0.1/16.0.3) |
| [@angular/material](https://github.com/angular/components) | dependencies | patch | [`16.0.1` -> `16.0.3`](https://renovatebot.com/diffs/npm/@angular%2fmaterial/16.0.1/16.0.3) |

---

### Release Notes

<details>
<summary>angular/components</summary>

### [`v16.0.3`](https://github.com/angular/components/blob/HEAD/CHANGELOG.md#&#8203;1603-silk-scarf-2023-06-01)

[Compare Source](angular/components@16.0.2...16.0.3)

##### cdk

| Commit | Type | Description |
| -- | -- | -- |
| [3d4a9d1607](angular/components@3d4a9d1) | fix | **overlay:** remove circular dependency workarounds ([#&#8203;27190](angular/components#27190)) |

##### material

| Commit | Type | Description |
| -- | -- | -- |
| [c8065d5168](angular/components@c8065d5) | fix | **chips:** fix click target when stacked ([#&#8203;27129](angular/components#27129)) |
| [12e47478c5](angular/components@12e4747) | fix | **slider:** handle ngModel initial null value ([#&#8203;27149](angular/components#27149)) |

<!-- CHANGELOG SPLIT MARKER -->

### [`v16.0.2`](https://github.com/angular/components/blob/HEAD/CHANGELOG.md#&#8203;1602-plastic-car-2023-05-25)

[Compare Source](angular/components@16.0.1...16.0.2)

##### cdk

| Commit | Type | Description |
| -- | -- | -- |
| [a0aa14fe1a](angular/components@a0aa14f) | fix | **schematics:** handle templates with byte order mark ([#&#8203;27131](angular/components#27131)) |

##### material

| Commit | Type | Description |
| -- | -- | -- |
| [82550af4d7](angular/components@82550af) | fix | **checkbox:** set token values on the element where theme is [@&#8203;included](https://github.com/included) ([#&#8203;27114](angular/components#27114)) |
| [d2e8a55b3b](angular/components@d2e8a55) | fix | **core:** Fix MacOS Hover feature compatibility with optgroup ([#&#8203;27081](angular/components#27081)) |
| [6dd2c605b1](angular/components@6dd2c60) | fix | **datepicker:** set explicit event options ([#&#8203;27082](angular/components#27082)) |
| [5c3107ccc6](angular/components@5c3107c) | fix | **icon:** harness returning wrong name if icon has other content ([#&#8203;27132](angular/components#27132)) |
| [9d9ceb2b22](angular/components@9d9ceb2) | fix | **list:** input coercion does not working for `disableRipple` on list items ([#&#8203;27126](angular/components#27126)) |
| [5aad5df78e](angular/components@5aad5df) | fix | **paginator:** add aria-hidden="true" to decorative svg's ([#&#8203;27083](angular/components#27083)) |

##### multiple

| Commit | Type | Description |
| -- | -- | -- |
| [12474aafc5](angular/components@12474aa) | fix | set aria-hidden on presentational svgs ([#&#8203;27116](angular/components#27116)) |

<!-- CHANGELOG SPLIT MARKER -->

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xMTEuMCIsInVwZGF0ZWRJblZlciI6IjM1LjExMS4wIiwidGFyZ2V0QnJhbmNoIjoiZGV2ZWxvcCJ9-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1918
Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
@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 Jul 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Accessibility This issue is related to accessibility (a11y) action: merge The PR is ready for merge by the caretaker area: material/chips dev-app preview When applied, previews of the dev-app are deployed to Firebase target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(MatChipSet): several chipset issues: clickable area, unselect, keyboard access
2 participants