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.

Sign up for GitHub

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

Jump to bottom

fix(material/progress-spinner): unable to change mode on spinner directive #14514

Merged
merged 1 commit into from
Feb 24, 2022
Merged

fix(material/progress-spinner): unable to change mode on spinner directive #14514

merged 1 commit into from
Feb 24, 2022

Conversation

crisbeto
Copy link
Member

Currently we have the mat-spinner directive which is a shortcut to a mat-progress-spinner with mode="indeterminate". Since the spinner inherits all of the inputs from the progress spinner, there's nothing stoping people from changing the mode back to determinate, however the element will look half-broken because the host bindings assume that the mode won't change. These changes update the host bindings to allow switching between modes.

Fixes #14511.

@crisbeto crisbeto added the target: patch This PR is targeted for the next patch release label Dec 14, 2018
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Dec 14, 2018
jelbourn
jelbourn reviewed Dec 19, 2018
View reviewed changes
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.

If we're going to do this, why not just just get rid of the extra component entirely and include both selectors in one? Then set the default value for mode based on the nodeName

@crisbeto crisbeto force-pushed the 14511/spinner-mode-override branch from c7fbb43 to 908df0e Compare December 19, 2018 07:14
@crisbeto
Copy link
Member Author

Good point. I've reworked it to only have the one class.

@crisbeto crisbeto force-pushed the 14511/spinner-mode-override branch 3 times, most recently from 3eeb918 to c97bdb1 Compare December 20, 2018 18:52
jelbourn
jelbourn approved these changes Dec 22, 2018
View reviewed changes
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 pr: lgtm action: merge The PR is ready for merge by the caretaker labels Dec 22, 2018
@ngbot
Copy link

ngbot bot commented Dec 22, 2018

I see that you just added the pr: merge ready label, but the following checks are still failing:
    failure status "ci/circleci: tests_browserstack" is failing
    failure status "ci/circleci: tests_saucelabs" is failing

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

@crisbeto crisbeto force-pushed the 14511/spinner-mode-override branch from c97bdb1 to 4084b55 Compare December 25, 2018 14:18
@crisbeto crisbeto force-pushed the 14511/spinner-mode-override branch from 4084b55 to 23aae02 Compare February 24, 2019 18:16
@crisbeto crisbeto force-pushed the 14511/spinner-mode-override branch 2 times, most recently from 5d30649 to 3dbcf7e Compare March 10, 2019 21:28
@crisbeto crisbeto force-pushed the 14511/spinner-mode-override branch from 3dbcf7e to 28d8c3f Compare April 14, 2019 17:18
@mmalerba mmalerba added aaa and removed aaa labels Apr 25, 2019
@crisbeto crisbeto force-pushed the 14511/spinner-mode-override branch from 28d8c3f to 88878b3 Compare May 5, 2019 13:10
@jelbourn
Copy link
Member

@crisbeto some teams have unit tests that depend on the presence of the .mat-spinner class; could you add that back?

@crisbeto crisbeto force-pushed the 14511/spinner-mode-override branch from 88878b3 to 1370735 Compare May 13, 2019 15:50
@crisbeto
Copy link
Member Author

Done.

@andrewseguin andrewseguin added the P4 A relatively minor issue that is not relevant to core functions label May 30, 2019
@serval42
Copy link

Hello,
We had tested a mat-spinner with mode="determinate" and a value. So the progress-bar turn and have the width of the percentile of progress. We find this comportment is great.
Can we keep or formalize this 'feature' ?

@crisbeto crisbeto force-pushed the 14511/spinner-mode-override branch from 1370735 to 46b0cb1 Compare October 6, 2019 18:02
@josephperrott josephperrott removed their assignment Oct 15, 2019
@crisbeto crisbeto force-pushed the 14511/spinner-mode-override branch from 46b0cb1 to 26867eb Compare December 31, 2019 06:09
@crisbeto crisbeto force-pushed the 14511/spinner-mode-override branch from 26867eb to d73e822 Compare February 7, 2020 21:54
@crisbeto crisbeto force-pushed the 14511/spinner-mode-override branch from d73e822 to 2a05b28 Compare March 14, 2020 20:15
@crisbeto crisbeto force-pushed the 14511/spinner-mode-override branch from 2a05b28 to 9a97b7b Compare May 10, 2020 14:08
@crisbeto crisbeto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent and removed P4 A relatively minor issue that is not relevant to core functions labels Jul 25, 2020
@crisbeto crisbeto force-pushed the 14511/spinner-mode-override branch from 9a97b7b to 5d0698e Compare July 25, 2020 06:27
@annieyw annieyw mentioned this pull request Jul 27, 2020
Merged
@mmalerba mmalerba removed the lgtm label Jul 31, 2020
@josephperrott josephperrott removed their request for review April 29, 2021 15:58
@andrewseguin andrewseguin added needs rebase and removed cla: yes PR author has agreed to Google's Contributor License Agreement labels Dec 29, 2021
@crisbeto crisbeto changed the title fix(progress-spinner): unable to change mode on spinner directive fix(material/progress-spinner): unable to change mode on spinner directive Feb 24, 2022
@crisbeto crisbeto force-pushed the 14511/spinner-mode-override branch from 5d0698e to 8eb98ca Compare February 24, 2022 09:01
@crisbeto crisbeto requested review from a team and andrewseguin as code owners February 24, 2022 09:01
@crisbeto
fix(material/progress-spinner): unable to change mode on spinner dire…
326efe3 
…ctive

Currently we have the `mat-spinner` directive which is a shortcut to a `mat-progress-spinner` with `mode="indeterminate"`. Since the spinner inherits all of the inputs from the progress spinner, there's nothing stoping people from changing the mode back to `determinate`, however the element will look half-broken because the host bindings assume that the mode won't change. These changes update the host bindings to allow switching between modes.

Fixes angular#14511.
@crisbeto crisbeto force-pushed the 14511/spinner-mode-override branch from 8eb98ca to 326efe3 Compare February 24, 2022 16:23
@crisbeto crisbeto merged commit 7480e3b into angular:master Feb 24, 2022
crisbeto added a commit that referenced this pull request Feb 24, 2022
@crisbeto
fix(material/progress-spinner): unable to change mode on spinner dire…
ea07fa8 
…ctive (#14514)

Currently we have the `mat-spinner` directive which is a shortcut to a `mat-progress-spinner` with `mode="indeterminate"`. Since the spinner inherits all of the inputs from the progress spinner, there's nothing stoping people from changing the mode back to `determinate`, however the element will look half-broken because the host bindings assume that the mode won't change. These changes update the host bindings to allow switching between modes.

Fixes #14511.

(cherry picked from commit 7480e3b)
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Mar 8, 2022
Update angularmaterial monorepo to v13.2.5 (#1199)
2a3e989 
This PR contains the following updates:

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

---

### Release Notes

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

### [`v13.2.5`](https://github.com/angular/components/blob/HEAD/CHANGELOG.md#&#8203;1325-satin-sash-2022-03-02)

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

##### cdk

| Commit | Type | Description |
| -- | -- | -- |
| [9e34a0f69f](angular/components@9e34a0f) | fix | **drag-drop:** error if preview dimensions are accessed too early ([#&#8203;24498](angular/components#24498)) |
| [9be3c46b01](angular/components@9be3c46) | fix | **testing:** TestElement sendKeys method should throw if no keys have been specified ([#&#8203;18271](angular/components#18271)) |
| [8e57a89cba](angular/components@8e57a89) | perf | **overlay:** add event listeners for overlay dispatchers outside of zone ([#&#8203;24408](angular/components#24408)) |

##### material

| Commit | Type | Description |
| -- | -- | -- |
| [ed2f516401](angular/components@ed2f516) | fix | **autocomplete:** auto-highlighted first option not display correctly if the floating label is disabled ([#&#8203;14507](angular/components#14507)) |
| [502102116e](angular/components@5021021) | fix | **autocomplete:** don't block default arrow keys when using modifiers ([#&#8203;11987](angular/components#11987)) |
| [f31fd3f066](angular/components@f31fd3f) | fix | **autocomplete:** reopen panel on input click ([#&#8203;16020](angular/components#16020)) |
| [5a79042d7d](angular/components@5a79042) | fix | **button-toggle:** use solid border color ([#&#8203;14253](angular/components#14253)) |
| [e2d4eecfcb](angular/components@e2d4eec) | fix | **checkbox:** inconsistent disabled color ([#&#8203;23083](angular/components#23083)) |
| [005ec323de](angular/components@005ec32) | fix | **checkbox:** incorrect text color when placed inside an overlay with a dark theme ([#&#8203;19054](angular/components#19054)) |
| [d7cbd1315f](angular/components@d7cbd13) | fix | **datepicker:** matDatepickerParse error not being added on first invalid value ([#&#8203;11524](angular/components#11524)) |
| [046022f31d](angular/components@046022f) | fix | **datepicker:** use aria-live over cdkAriaLive on period button ([#&#8203;24398](angular/components#24398)) |
| [37f69dbf7e](angular/components@37f69db) | fix | **dialog:** use passed in ComponentFactoryResolver to resolve dialog content ([#&#8203;17710](angular/components#17710)) |
| [2e15f54a9f](angular/components@2e15f54) | fix | **menu:** focus lost if active item is removed ([#&#8203;14039](angular/components#14039)) |
| [ea07fa8e64](angular/components@ea07fa8) | fix | **progress-spinner:** unable to change mode on spinner directive ([#&#8203;14514](angular/components#14514)) |
| [1a498a6a81](angular/components@1a498a6) | fix | **sort:** remove role from header when disabled ([#&#8203;24477](angular/components#24477)) |
| [72019531db](angular/components@7201953) | fix | **tooltip:** don't hide when pointer moves to tooltip ([#&#8203;24475](angular/components#24475)) |

##### material-experimental

| Commit | Type | Description |
| -- | -- | -- |
| [7b85cc077c](angular/components@7b85cc0) | fix | **mdc-button:** density styles being overwritten by structural styles ([#&#8203;22736](angular/components#22736)) |
| [aeb1426e4c](angular/components@aeb1426) | fix | **mdc-chips:** expose avatar harness ([#&#8203;24499](angular/components#24499)) |

#### Special Thanks

Andrew Seguin, Artur Androsovych, Jeri Peier, Kristiyan Kostadinov, Paul Gschwendtner, Yousaf Nawaz and Zach Arend

<!-- 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 <[email protected]>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1199
Reviewed-by: Epsilon_02 <[email protected]>
Co-authored-by: Calciumdibromid Bot <[email protected]>
Co-committed-by: Calciumdibromid Bot <[email protected]>
@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 Mar 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jelbourn jelbourn jelbourn approved these changes

@andrewseguin andrewseguin Awaiting requested review from andrewseguin

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

mat-spinner mode="determinate", but it is whirls constantly
7 participants
@crisbeto @jelbourn @serval42 @googlebot @josephperrott @mmalerba @andrewseguin