Skip to content
This repository was archived by the owner on Oct 5, 2019. It is now read-only.

Conversation

@ajmirsky
Copy link
Contributor

@ajmirsky ajmirsky commented Jan 5, 2016

angular directive's link function gets 'attrs' with normalized names (ie lowercase, without dashes) even though in the local it's case sensative. also, send the name of the event along with the event value to the callback

…(ie lowercase, without dashes) even though in the local it's case sensative. also, send the name of the event along with the event value to the callback
@seiyria
Copy link
Owner

seiyria commented Jan 6, 2016

Thanks!

seiyria added a commit that referenced this pull request Jan 6, 2016
event callbacks not triggered due to attribute naming issue
@seiyria seiyria merged commit 0772845 into seiyria:master Jan 6, 2016
@knstntn
Copy link
Contributor

knstntn commented Jan 25, 2016

lowercase

Is that true? I checked angular source code and they seem to be using camelCase for normalization. Cannot find any details about lowercase in angular docs either.

This change seems incorrect to me. Original usage of the attribute was "on-stop-slide='callback($event, value)'" (this will be normalized to "onStopSlide"), while now I have to update code and use "onStopSlide='callback($event, value)'" which is not very angularish (but which indeed will be converted to lowercase because it does not contain dashes).

Anyway, this change can be left as is, but at least could you update version of the script, so I could use old behavior and not update my pages?

@seiyria
Copy link
Owner

seiyria commented Jan 25, 2016

The version is updated. I'd accept a PR that reverts this though, because you're right, this is not very angular-y. I'm not sure why the event here got changed though, which is something I'd like info on before anything happens.

@knstntn
Copy link
Contributor

knstntn commented Jan 25, 2016

Yeah, I wanted to make a PR, but I was confused with this part as well. Before the eventArgs were sent, but not it is plain text. And again, this kind of change is breaking change, someone already might be using new version with new approach

(Example is wrong too, it should be on-stop-slide, not on-slide-stop. Or even onStopSlide considering current code ;))

@seiyria
Copy link
Owner

seiyria commented Jan 25, 2016

Yeah, sorry. I don't have many tests and I don't actually use this anymore. I'd be open to having someone else help maintain it because things like this should not happen, but it's hard to keep an eye on things when I'm not actively using it.

@knstntn knstntn mentioned this pull request Jan 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants