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 aria live announcer #238

Merged
merged 1 commit into from
Apr 6, 2016

Conversation

devversion
Copy link
Member

Fixes #106

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Mar 28, 2016
@devversion devversion force-pushed the wip/aria-announcer branch 3 times, most recently from 56be961 to a3836d5 Compare March 29, 2016 14:32

// This hides the announcer element visually.
// That means it's still accessible for screen-readers but not visible.
._md-aria-announcer {
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 omit the underscore from the name.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes right, seems to be unnecessary, because of the view encapsulation.

@jelbourn
Copy link
Member

cc @marcysutton in case you're interested :)

@devversion devversion force-pushed the wip/aria-announcer branch 7 times, most recently from a2d1ccd to 1e55ee7 Compare March 30, 2016 14:24
@@ -0,0 +1,37 @@
# MdAriaLive
Copy link
Member

Choose a reason for hiding this comment

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

The readme still uses aria-live

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, shameful. I was sure to change that.

@devversion devversion force-pushed the wip/aria-announcer branch 2 times, most recently from 62faa49 to d1acbaf Compare April 3, 2016 12:57
@jelbourn
Copy link
Member

jelbourn commented Apr 5, 2016

@devversion sorry to change course on you with this PR, but I talked with hansl@ and we realized that doing this as a service would actually make more sense.

  • It ensures that there's only one announcer globally
  • It can be injected anywhere in the app without having to be a parent of anything

The service would be responsible for making its own DOM element and appending it to the document body (and also checking whether there is already an announcer element present).

@devversion
Copy link
Member Author

@jelbourn That's no problem. I will change that as soon as possible and ping you. (lot of stress atm)

@jelbourn
Copy link
Member

jelbourn commented Apr 5, 2016

@devversion No worries! We don't have any immediate need for this.

@devversion devversion force-pushed the wip/aria-announcer branch from d1acbaf to 7b52d5a Compare April 6, 2016 14:07
@devversion devversion force-pushed the wip/aria-announcer branch from 9127440 to 72f9fef Compare April 6, 2016 15:06
@devversion
Copy link
Member Author

@jelbourn Updated it. Please take a look again.

@Injectable()
export class MdLiveAnnouncer {

private announcerEl: Element;
Copy link
Member

Choose a reason for hiding this comment

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

How about _liveElement ?

@devversion devversion force-pushed the wip/aria-announcer branch from 72f9fef to 3883045 Compare April 6, 2016 17:37
@jelbourn
Copy link
Member

jelbourn commented Apr 6, 2016

LGTM

@jelbourn
Copy link
Member

jelbourn commented Apr 6, 2016

@devversion Just notices one last thing from CI: IE11 doesn't support Element.prototype.remove. Once that's resolved I can merge

@devversion devversion force-pushed the wip/aria-announcer branch from 3883045 to 96536d9 Compare April 6, 2016 18:16
@jelbourn jelbourn merged commit e99da66 into angular:master Apr 6, 2016
@devversion devversion deleted the wip/aria-announcer branch April 6, 2016 18:26
devversion added a commit to devversion/material that referenced this pull request Oct 3, 2016
* Removes the static messages, which should be detected by the screenreaders

* Introduces a Screenreader Announcer service (as in Material 2 - angular/components#238)

* Service can be used for other components as well (e.g Toast, Tooltip)

Fixes angular#9603.
kara pushed a commit to angular/material that referenced this pull request Oct 12, 2016
* Removes the static messages, which should be detected by the screenreaders

* Introduces a Screenreader Announcer service (as in Material 2 - angular/components#238)

* Service can be used for other components as well (e.g Toast, Tooltip)

Fixes #9603.
@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 5, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build aria-live announcer service / component
5 participants