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.

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

feat(): add progress-fab component #290

Closed

Conversation

devversion
Copy link
Member

The feature is still under discussion. just decided to push it, so others can review and suggest improvements.

https://i.gyazo.com/4cb5fe4e70c4845e58d1c382d95f1eed.mp4


MdProgressFab

MdProgressFab is a normal FAB button surrounded with a progress-circle.

Screenshots

Preview

[md-progress-fab]

Bound Properties

Name Type Description
color `"primary" "accent"
value number Value for the progress-circle.
Necessary when using the determinate mode.
mode `"determinate" "indeterminate"`
progressColor `"primary" "accent"

Examples

A basic progress-fab will have the markup:

<button md-progress-fab color="accent">
  <i class="material-icons md-24">favorite</i>
  </button>

It will use by default a indeterminate progress circle.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Apr 9, 2016
@jelbourn jelbourn added the in progress This issue is currently in progress label Apr 11, 2016
@devversion devversion changed the title Feat/progress fab component feat(): add progress-fab component Apr 13, 2016
@devversion devversion force-pushed the feat/progress-fab-component branch 4 times, most recently from f16d70d to e821cea Compare April 14, 2016 13:53
@jelbourn
Copy link
Member

jelbourn commented May 4, 2016

@devversion Now needs rebasing on my mega change #384; feel free to ping me if you run into any issues

@devversion devversion force-pushed the feat/progress-fab-component branch from e821cea to 09e8561 Compare May 4, 2016 18:24
@devversion
Copy link
Member Author

@jelbourn Done.

@devversion devversion force-pushed the feat/progress-fab-component branch from 09e8561 to 3fb2ad1 Compare May 4, 2016 18:32
builder = tcb;
}));

it('should correctly apply the color attribute on the progress circle', (done: () => void) => {
Copy link
Member

Choose a reason for hiding this comment

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

You can use the new async function instead of the done callback now

@jelbourn
Copy link
Member

@devversion sorry for the delay on this PR, I need to set some time aside to see what the a11y story for this component should be.

@devversion
Copy link
Member Author

devversion commented May 16, 2016

@jelbourn It's no problem :)
There are currently other things with higher priorities.

Just ping me on the PR, once you like to revisit it.

<ng-content></ng-content>

<div class="md-progress-wrap">
<md-progress-circle

Choose a reason for hiding this comment

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

To be announced, this progress circle needs to be a sibling to the focusable button, and it needs role="progressbar" along with other related ARIA attributes: https://www.w3.org/TR/wai-aria/roles#progressbar

Specifically the progressbar needs to specify aria-valuenow, aria-valuemin and aria-valuemax for screen readers to follow along.

Lastly, what is the progress circle indicating (i.e. what is loading)? That binding should be indicated somehow...the ARIA roles spec recommends using aria-describedby on the progressbar and aria-busy on whatever is loading.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @marcysutton, I'll try making the process circle a sibling.

  • The role is automatically applied from the Progress Circle Component
  • Valuenow, ValueMin, ValueNow are also covered by the circle component.

Forwarding the aria-label / or creating a default one, makes definitely sense. thx

@marcysutton
Copy link

To answer @jelbourn's question from #305, this definitely does need some a11y treatment. While the button is the focusable element, the progressbar component (which needs ARIA) should be a sibling so it doesn't live inside of the button. That way the progressbar can be consumed effectively by assistive technologies, instead of trying to announce it in the context of a button (which has a different purpose and function).

If the progressbar component can't bind itself to whatever is actually loading on the page with aria-describedby, you'll have to make that step clear in the component documentation.

@devversion
Copy link
Member Author

Going to close this for now, since this is a nice to have component

@devversion devversion closed this Sep 20, 2016
@devversion devversion deleted the feat/progress-fab-component branch September 20, 2016 18:21
@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 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes PR author has agreed to Google's Contributor License Agreement in progress This issue is currently in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants