-
Notifications
You must be signed in to change notification settings - Fork 0
Implement progress button #1
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
Conversation
andrewseguin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will add some specificity that I'd expect to be somewhat breaking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh that is a very interesting idea, I will try it and see what happens
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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