Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

feat(ngAria): add support for ngReadonly#14140

Closed
m-amr wants to merge 1 commit into
angular:masterfrom
m-amr:add_ariaReadonly
Closed

feat(ngAria): add support for ngReadonly#14140
m-amr wants to merge 1 commit into
angular:masterfrom
m-amr:add_ariaReadonly

Conversation

@m-amr

@m-amr m-amr commented Feb 27, 2016

Copy link
Copy Markdown
Contributor

handle aria-readonly based on ng-readonly
fix #14077

@m-amr m-amr force-pushed the add_ariaReadonly branch 3 times, most recently from 1de0bee to 008fe0c Compare February 27, 2016 02:14
@Narretz Narretz added this to the 1.5.x milestone Feb 27, 2016
Comment thread test/ngAria/ariaSpec.js

scope.$apply('val = true');
expect(element.attr('aria-readonly')).toBe('true');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nitpick: Redundant newline.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have removed the Nitpick: Redundant newline.

@gkalpak

gkalpak commented Feb 28, 2016

Copy link
Copy Markdown
Member

The code LGTM. Thx for working on this 👍
There are some missing docs:

  1. Mention ngReadonly in the full list of directives that interface with ngAria.
  2. Mention ngReadonly in the accessibiity dection of the Dev Guide (file: docs/content/guide/accessibility.ngdoc). See fix(ngAria): Apply ARIA attrs correctly #13483 for an example.

    handle aria-readonly based on ng-readonly
    feat   angular#14077
@m-amr

m-amr commented Feb 28, 2016

Copy link
Copy Markdown
Contributor Author

I have updated the documentation to include
1 - full list of directives that interface with ngAria.
2 - docs/content/guide/accessibility.ngdoc

Thanks for your feedback.

@Narretz

Narretz commented Mar 30, 2016

Copy link
Copy Markdown
Contributor

@gkalpak can you give this another look / merge it? It LGTM

@Narretz Narretz self-assigned this Apr 7, 2016
Narretz pushed a commit that referenced this pull request Apr 8, 2016
@Narretz Narretz closed this in ae0a716 Apr 8, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(ngAria): add support for ngReadonly

4 participants