Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

WIP: Refactor to use ngAria #316

Closed
wants to merge 4 commits into from
Closed

Conversation

btford
Copy link
Contributor

@btford btford commented Sep 23, 2014

Still getting some tests to pass; we might need to make some changes to ngAria to get the material switch component to work.

/cc @marcysutton

@btford btford added the a11y This issue is related to accessibility label Sep 23, 2014

function render() {
checked = ngModelCtrl.$viewValue;
element.attr(Constant.ARIA.PROPERTY.CHECKED, checked);
Copy link
Contributor

Choose a reason for hiding this comment

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

With removal of this line, aria-checked isn't being added even though ngAria is in place. Some bugs to iron out there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, not sure how to go about this. also thinking if this model of composition for directives really makes sense. might be better to have a few different linking-like fns and mix them in to each individual directive.

Copy link
Contributor

Choose a reason for hiding this comment

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

There does seem to be repetition amongst some of the Material directives. But the interaction models and required ARIA attributes amongst components are quite varied. What behaviors would you move into link functions?

@marcysutton
Copy link
Contributor

I'm getting test failures when I run this branch:
Error: [$injector:modulerr] Failed to instantiate module ngAria due to:
Error: [$injector:nomod] Module 'ngAria' is not available!

@btford
Copy link
Contributor Author

btford commented Sep 23, 2014

did you bower install ?

@marcysutton
Copy link
Contributor

Derp! I did, but then I switched branches. Thanks.

@btford
Copy link
Contributor Author

btford commented Sep 23, 2014

haha it happens

@marcysutton
Copy link
Contributor

Still seeing the same problem after bower install. I think it's due to this:
bower ENORESTARGET Tag/branch ~1.3.0-build.3289+sha.d8c8b2e does not exist

@btford
Copy link
Contributor Author

btford commented Sep 24, 2014

weiiird. lemme try bumping the deps.

@btford
Copy link
Contributor Author

btford commented Sep 24, 2014

@marcysutton – can pull and try with 31898cb ?

@marcysutton
Copy link
Contributor

@btford works! I'll get started on some tests.

@btford
Copy link
Contributor Author

btford commented Sep 24, 2014

great. thanks!

@marcysutton
Copy link
Contributor

@btford I'm wondering if we should recreate this PR in the material repo so that it's easier to rebase/merge. Right now I have to experiment with it as a separate fork.

@marcysutton
Copy link
Contributor

@btford I cherry-picked these commits into the Material repo. Closing this PR!

@btford
Copy link
Contributor Author

btford commented Sep 26, 2014

thx!

On Fri, Sep 26, 2014 at 2:34 PM, Marcy Sutton notifications@github.com
wrote:

@btford https://github.com/btford I cherry-picked these commits into
the Material repo. Closing this PR!


Reply to this email directly or view it on GitHub
#316 (comment).

@cristiano-belloni
Copy link

I'm still getting ngAria errors:

Error: [$injector:modulerr] Failed to instantiate module ngMaterial due to:
Error: [$injector:modulerr] Failed to instantiate module ngAria due to:
Error: [$injector:nomod] Module 'ngAria' is not available! You either misspelled the module name or forgot to load it. If registering a module ensure that you specify the dependencies as the second argument.

My relevant bower.json entries are:

"angular-material": "master"
"angular": "1.3"

I bower-installed an bower ls says:

#1.3.0-rc.4
angular-material#914bed7916

So everything is updated, but I still get the ngAria problem.

@marcysutton
Copy link
Contributor

@janesconference This PR was closed and the remainder of the work was moved to the main Angular Material repo. Have you experienced this problem on master?

@cristiano-belloni
Copy link

@marcysutton yep, my bower entry is "angular-material": "master" and I'm currently at commit 914bed7916 (the last current one as I'm writing - angular/bower-material@914bed7).

@marcysutton
Copy link
Contributor

bower.json should include "angular-aria": "~1.3.0-rc.4" if you're on the latest commit on Angular master (4da5673). Confirmed that version of angular-aria exists and installs correctly.

@cristiano-belloni
Copy link

Yep, bower install angular-aria --save works. I don't know what was I thinking. Thank you.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a11y This issue is related to accessibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants