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

refactor(ngAria): DRY camelcase method #10338

Closed
wants to merge 1 commit into from

Conversation

cironunes
Copy link
Member

@petebacondarwin I'm using attr.$normalize as you suggested in #10337 (comment)

The problem is that we also need to normalize the name in the service. Probably that's why the local camelCase method was created in the first place.

I still think that we should have a public camelcaseas we do with lowercase and uppercase, but I'm not sure how to better approach this, what do you think?

@googlebot
Copy link

CLAs look good, thanks!

@petebacondarwin
Copy link
Contributor

That is also a bug in ngAria. They should not be handling non-canonical attribute names in $ariaProvider.config at https://github.com/angular/angular.js/blob/master/src/ngAria/aria.js#L181.

And in any case if they were then they should be using the directiveNormalize version.

Instead, they should require that this param is already normalized. In the code the only place this happens in ngAria is inside shouldAttachAttr, which should also always be called with a normalized attribute.

Some cases are already covered (hard-coded strings) except for at line 244, line 265, line 275, line 283.

@petebacondarwin
Copy link
Contributor

We want to limit the public API to "helper" methods since this adds to the size of the core file and the maintenance effort. Many of the angular.xxx methods should not have been public in the first place :-)

@cironunes
Copy link
Member Author

@petebacondarwin I updated the PR with the changes. Now the local camelCase can be removed. :)

@cironunes
Copy link
Member Author

I understand, thanks for the patience to explain.

@petebacondarwin
Copy link
Contributor

Thanks!!

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.

3 participants