Skip to content

demo(progress-spinner): Add progress spinner accessibility demo page #6347

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

Merged
merged 5 commits into from
Aug 9, 2017

Conversation

tinayuangao
Copy link
Contributor

No description provided.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Aug 8, 2017
@tinayuangao tinayuangao added Accessibility This issue is related to accessibility (a11y) pr: needs review labels Aug 8, 2017
<section>
<h2>Loading indicator (Indeterminate progress spinner)</h2>
<md-progress-spinner mode="indeterminate"
[value]="loadingValue" color="primary" [strokeWidth]="1">
Copy link
Contributor

Choose a reason for hiding this comment

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

do indeterminate ones need a value?


<section>
<h2>Potion of completion (Determinate progress spinner) </h2>
<md-progress-spinner mode="indeterminate" color="primary"></md-progress-spinner>
Copy link
Contributor

Choose a reason for hiding this comment

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

should be determinate? (also needs a value)

<h2>Potion of completion (Determinate progress spinner) </h2>
<md-progress-spinner mode="indeterminate" color="primary"></md-progress-spinner>
<md-spinner color="warn"></md-spinner>
</section>
Copy link
Contributor

Choose a reason for hiding this comment

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

\n

@tinayuangao
Copy link
Contributor Author

Comments addressed. Please take another look. Thanks for review!

<md-progress-spinner mode="indeterminate"
[value]="loadingValue" color="accent"></md-progress-spinner>
<md-progress-spinner mode="indeterminate" color="accent"></md-progress-spinner>
<md-spinner color="warn"></md-spinner>
</section>

<section>
<h2>Potion of completion (Determinate progress spinner) </h2>
Copy link
Contributor

Choose a reason for hiding this comment

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

did you mean "Portion"?

@@ -0,0 +1,16 @@
<section>
<h2>Loading indicator (Indeterminate progress spinner)</h2>
<md-progress-spinner mode="indeterminate" color="primary" [strokeWidth]="1">
Copy link
Member

Choose a reason for hiding this comment

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

Instead of <md-progress-spinner mode="indeterminate">, you can just do <md-spinner> (which is indeterminate by default)


<section>
<h2>Potion of completion (Determinate progress spinner) </h2>
<md-progress-spinner mode="determinate" color="primary"
Copy link
Member

Choose a reason for hiding this comment

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

Should be able to omit the mode for determinate since it's the default

@tinayuangao tinayuangao force-pushed the a11y-progress-spinner branch 2 times, most recently from a0077ff to f1cf872 Compare August 8, 2017 18:40
@tinayuangao
Copy link
Contributor Author

Comments addressed. Please take another look. Thanks for review!

@@ -0,0 +1,13 @@
<section>
<h2>Loading indicator (Indeterminate progress spinner)</h2>
<md-spinner color="indeterminate" [strokeWidth]="1"></md-spinner>
Copy link
Member

Choose a reason for hiding this comment

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

Also just occurred to me that each progress component should have an aria-label

@tinayuangao
Copy link
Contributor Author

Added aria-label to each progress spinner. Please take a look. Thanks!

<section>
<h2>Portion of completion (Determinate progress spinner) </h2>
<md-progress-spinner [strokeWidth]="1" color="primary"
[value]="portionValue" aria-label="Portion complete">
Copy link
Member

Choose a reason for hiding this comment

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

Could you make the labels something like "Portion of pizza eaten", "Portion of cake eaten"? (e.g. more "real life" examples)

Copy link
Contributor Author

@tinayuangao tinayuangao left a comment

Choose a reason for hiding this comment

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

Changed to "portion of pizza eaten". PTAL. Thanks!

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, add merge-ready when ready

[value]="portionValue" aria-label="Portion of pizza eaten">
</md-progress-spinner>
<md-progress-spinner color="warn" [value]="portionValue"
aria-label="Portion of pizza eate">
Copy link
Member

Choose a reason for hiding this comment

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

Missing n at the end of the label

@tinayuangao tinayuangao force-pushed the a11y-progress-spinner branch from dbb23b1 to e25b78c Compare August 9, 2017 17:57
@tinayuangao tinayuangao added the action: merge The PR is ready for merge by the caretaker label Aug 9, 2017
@mmalerba mmalerba merged commit 0e4680f into angular:master Aug 9, 2017
@tinayuangao tinayuangao deleted the a11y-progress-spinner branch March 9, 2018 23:31
@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 Sep 8, 2019
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 cla: yes PR author has agreed to Google's Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants