Skip to content

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Jun 5, 2021

Fixes that the button component was always setting a tabindex="0" when it's placed on a link element. This is unnecessary, because links are in the tab order by default.

We actually had a test that has an assertion against this, but it was passing by accident because we weren't running change detection before making the assertion.

@crisbeto crisbeto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: patch This PR is targeted for the next patch release labels Jun 5, 2021
@crisbeto crisbeto requested a review from andrewseguin June 5, 2021 10:55
@crisbeto crisbeto requested a review from jelbourn as a code owner June 5, 2021 10:55
@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jun 5, 2021
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn added the action: merge The PR is ready for merge by the caretaker label Jun 7, 2021
@wagnermaciel
Copy link
Contributor

@crisbeto This has a a merge conflict with master

@crisbeto crisbeto force-pushed the button-link-tabindex branch from 69f0124 to 7d3fb74 Compare June 11, 2021 05:24
@crisbeto
Copy link
Member Author

The merge conflict has been resolved.

@crisbeto crisbeto force-pushed the button-link-tabindex branch from 7d3fb74 to 69cdd5c Compare September 3, 2021 18:03
@andrewseguin andrewseguin removed the cla: yes PR author has agreed to Google's Contributor License Agreement label Dec 28, 2021
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Dec 29, 2021
@crisbeto crisbeto force-pushed the button-link-tabindex branch from 69cdd5c to c1f7b5d Compare January 3, 2022 09:18
Fixes that the button component was always setting a `tabindex="0"` when it's placed on a link element. This is unnecessary, because links are in the tab order by default.

We actually had a test that has an assertion against this, but it was passing by accident because we weren't running change detection before making the asserting.
@crisbeto crisbeto force-pushed the button-link-tabindex branch from c1f7b5d to 942dc24 Compare March 3, 2022 07:43
@crisbeto crisbeto merged commit 899d0e1 into angular:master Mar 4, 2022
crisbeto added a commit that referenced this pull request Mar 4, 2022
…22901)

Fixes that the button component was always setting a `tabindex="0"` when it's placed on a link element. This is unnecessary, because links are in the tab order by default.

We actually had a test that has an assertion against this, but it was passing by accident because we weren't running change detection before making the asserting.

(cherry picked from commit 899d0e1)
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Mar 18, 2022
This PR contains the following updates:

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

---

### Release Notes

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

### [`v13.2.6`](https://github.com/angular/components/blob/HEAD/CHANGELOG.md#&#8203;1326-suede-spaghetti-2022-03-09)

[Compare Source](angular/components@13.2.5...13.2.6)

##### cdk

| Commit | Type | Description |
| -- | -- | -- |
| [39929a815d](angular/components@39929a8) | fix | **overlay:** backdrop timeouts not being cleared in some cases ([#&#8203;23972](angular/components#23972)) |
| [2f2b0c7cf4](angular/components@2f2b0c7) | fix | **testing:** dispatch mouseover and mouseout events in UnitTestElement ([#&#8203;24490](angular/components#24490)) |
| [edca54f2d0](angular/components@edca54f) | fix | **testing:** require at least one argument for locator functions ([#&#8203;23619](angular/components#23619)) |

##### material

| Commit | Type | Description |
| -- | -- | -- |
| [c4993ac171](angular/components@c4993ac) | fix | **button:** avoid setting a tabindex on all link buttons ([#&#8203;22901](angular/components#22901)) |
| [c47d30e0e5](angular/components@c47d30e) | fix | **dialog:** don't wait for animation before moving focus ([#&#8203;24121](angular/components#24121)) |
| [70b8248568](angular/components@70b8248) | fix | **expansion:** able to tab into descendants with visibility while closed ([#&#8203;24045](angular/components#24045)) |
| [d22d73ab8d](angular/components@d22d73a) | fix | **select:** disabled state out of sync when swapping form group with a disabled one ([#&#8203;17872](angular/components#17872)) |
| [911d6b71d4](angular/components@911d6b7) | fix | **slide-toggle:** clear name from host node ([#&#8203;15505](angular/components#15505)) |
| [4b5363d160](angular/components@4b5363d) | fix | **tooltip:** decouple removal logic from change detection ([#&#8203;19432](angular/components#19432)) |

##### material-experimental

| Commit | Type | Description |
| -- | -- | -- |
| [8414646d79](angular/components@8414646) | fix | **mdc-card:** remove extra margin if header doesn't have an avatar ([#&#8203;19072](angular/components#19072)) |
| [f66486dc5b](angular/components@f66486d) | fix | **mdc-slider:** fix a few null pointer exceptions ([#&#8203;23659](angular/components#23659)) |

##### multiple

| Commit | Type | Description |
| -- | -- | -- |
| [6ee0089ce6](angular/components@6ee0089) | fix | don't block child component animations on open ([#&#8203;24529](angular/components#24529)) |

#### Special Thanks

Andrew Seguin, Jeri Peier, Kristiyan Kostadinov and Paul Gschwendtner

<!-- CHANGELOG SPLIT MARKER -->

</details>

---

### Configuration

📅 **Schedule**: 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, click this checkbox.

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1214
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>
forsti0506 pushed a commit to forsti0506/components that referenced this pull request Apr 3, 2022
…ngular#22901)

Fixes that the button component was always setting a `tabindex="0"` when it's placed on a link element. This is unnecessary, because links are in the tab order by default.

We actually had a test that has an assertion against this, but it was passing by accident because we weren't running change detection before making the asserting.
@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 Apr 4, 2022
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 cla: yes PR author has agreed to Google's Contributor License Agreement P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants