search

PolymerElements / iron-selector

Manages a list of elements that can be selected
32 stars 55 forks source link

Fixes #57: `attr-for-selected` now always expects hyphenated names. #109

Closed bicknellr closed 8 years ago

bicknellr commented 8 years ago

Continuation of #101. (Fixes #57.)

notwaldorf commented 8 years ago

Is this a breaking change? Like, did it use to work with both and now it doesn't?

bicknellr commented 8 years ago

Hmm. Previously, the behavior for finding the item's key was pretty much item[attrForSelected] || item.getAttribute(attrForSelected). You could reference attributes or properties, but not both, because of the different cases. If I'm reading dashToCamelCase's implementation right, it shouldn't actually change the behavior for passing camel case strings since dashToCamelCase won't actually modify it. So, this change actually handles more cases but adds (unenforced) restrictions to the API. <- I'd say minor version bump for this reason, even though this should still work in all the cases we had before without changes.

p.s. I just noticed dashToCamelCase does its own caching, but it has to do a lookup rather than just one comparison; do you think this makes _lastAttrForSelected/_lastPropertyNameForSelected stuff here overkill?

notwaldorf commented 8 years ago

@bicknellr Yeaaah i'd say lookups are free, so maybe keep it cleaner and don't do your own caching?

bicknellr commented 8 years ago

Ok, I removed the caching, so it's just a call to dashToCamelCase now.

notwaldorf commented 8 years ago

LGTM if the comment becomes more of a warning/advice :)

notwaldorf commented 8 years ago

👌