Conversation
andrewseguin
left a comment
There was a problem hiding this comment.
It feels like this is ultimately the same (or more) work for a user than just putting in something like:
<button matButton>
@if (loading) {
<mat-progress-spinner
progressIndicator
mode="indeterminate"
diameter="20"
aria-label="Loading"
tabindex=""
/>
} @else { Submit }
</button>
The restriction of not letting button depend on the progress spinner is what really prevents this from being actually ergonomic. Instead I think this is adding a pretty specific incantation to mimic the more straightforward HTML listed above
|
@andrewseguin not quite, the changes also ensure that the layout is correct, that the content doesn't shift, and that the icons and label are hidden. This would be rather painful for everyone to implement themselves, see the many attempts in g3 |
|
Oh just tried with icons: Angular takes the @else block and stuffs it into the main Let's move forward but we do need to fix that tabindex bit, or just make sure its clearly documented as requiring |
…nside the button Add a new API, testing API, and docs for allowing progress indicators to be projected into a button and have them shown in an accessible manner Fixes angular#13667
bd4c6fb to
3352a2c
Compare
|
|
||
| img, | ||
| svg { | ||
| svg:not(.mat-mdc-button-progress-indicator-container *) { |
There was a problem hiding this comment.
This will add some specificity that I'd expect to be somewhat breaking.
There was a problem hiding this comment.
Is there a better way to handle this? We want to keep applying these styles to the SVGs, but if we include these styles to the SVG in MatProgressSpinner then the MatProgressSpinner breaks
| > | ||
| </ng-content> | ||
|
|
||
| @if (showProgress()) { |
There was a problem hiding this comment.
Rather than introducing this @if, could we get the same result by changing the selectors to either .mat-mdc-button-progress-indicator-container:not(:empty) or .mat-mdc-button-progress-indicator-container:has(mat-progress-spinner)?
There was a problem hiding this comment.
Oh that is a very interesting idea, I will try it and see what happens
There was a problem hiding this comment.
So essentially with this approach we don't have the input at all, we just have content projected and control via CSS?
If so then I think this wont work because we need to change the classes outside of just the progress indicator to make things invisible
There was a problem hiding this comment.
As in, we need to know that the progress treatment is being requested so that we:
- Show the element for the progress indicator
- Hide the label and the icons which are outside of the progress indicator element itself
|
@crisbeto and @andrewseguin Let's take the discussion over to angular#32698 which is the more legit form of this PR |
Dummy PR for discussion, many open questions still