Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

docs(accordion): update breaking changes #5306

Closed
wants to merge 1 commit into from
Closed

docs(accordion): update breaking changes #5306

wants to merge 1 commit into from

Conversation

trickpattyFH20
Copy link
Contributor

No description provided.

@icfantv
Copy link
Contributor

icfantv commented Jan 19, 2016

Technically, we should add this to all directives that use replace: true on their templates...

@wesleycho
Copy link
Contributor

I don't know if I'd classify this as a breaking change - this was always unsupported behavior by Angular. That it worked when using UI Bootstrap was more incidental.

@trickpattyFH20
Copy link
Contributor Author

I 100% understand that it was an incidental bug, not a feature.
But regardless of how minute/incidental it was, it is now definitely a breaking change.

@wesleycho
Copy link
Contributor

That is not what is considered a breaking change - a breaking change is one that breaks supported behavior.

@trickpattyFH20
Copy link
Contributor Author

hmmm... well, agree to disagree I guess. Call it what you may(workaround, incident, bug, feature etc.) but this behavior used to be supported by UI Bootstrap, and it is no longer supported.

@icfantv
Copy link
Contributor

icfantv commented Jan 20, 2016

@trickpattyFH20, i think it was supported by pure luck. i'm sorry you're frustrated and you have to change your code to make this work correctly. i do hope you can see that our hands are essentially tied.

@trickpattyFH20
Copy link
Contributor Author

@icfantv I just don't think it's very descriptive or accurate to call it "lucky" or "incidental"

It is irrelevant that Angular has never supported a replace:true directive with ng-class in both places. UIB used to support this behavior by using the interpolated class workaround, and now it doesn't. I consider this a breaking change to the UIB library and feel that it deserves to be documented. Whether or not it used to be a bug or a feature of the UIB library is the only thing that could be argued in my mind. That's the last I'll say about it.

@Foxandxss
Copy link
Contributor

No, breaking changes are only for things that we support. It is not something we made up, you can see any other changelog in any other project.

Let me put an example. We used to test all our animations in a certain way, never documented, never "official". One day angular decided to remove that method so we need to update all our tests. That never appeared on their changelog, since that was never documented as a feature you can use.

About the concrete issue, it is not really a ui-boostrap issue, it is more of an angular issue, so we cannot document all the weird behaviors someone can encounter because that would be a big pain to maintain.

Thanks for the heads up tho, we opened an issue at the angular repo angular/angular.js#13805 but this doesn't seem to go anywhere else.

@Foxandxss Foxandxss closed this Jan 20, 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.

4 participants