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

feat(carousel): use uib- prefix #4501

Closed
wants to merge 1 commit into from
Closed

feat(carousel): use uib- prefix #4501

wants to merge 1 commit into from

Conversation

BobbieBarker
Copy link
Contributor

Added uib prefixing to the carousel directive.

@wesleycho
Copy link
Contributor

Clean up the history (there should not be a merge commit) - also we need tests for the deprecation warning.

@BobbieBarker
Copy link
Contributor Author

i knew I was forgetting something!

@Foxandxss
Copy link
Contributor

There is a list of things that needs to be fixed:

Slide needs prefixing
You are creating the carousel module twice
Remove the controller from the deprecated part
Remove the animation from the deprecated part
Make the deprecated carousel use the prefixed controller
Fix the deprecation comment to use carousel
Change $alertSupressWarning to collapse (in all different places)
Add tests for deprecation warning
Fix all the tests to use the new prefixed versions

Perhaps I am forgetting something, but that is a good start.

@Foxandxss
Copy link
Contributor

Nice werk. Remaining.

Remove all the extra docs on the deprecated part (I want to remove them all but that is a different topic).
The deprecated slide needs deprecation messages too
You still need to change the $alertSuppress thingy to carousel.

@Foxandxss
Copy link
Contributor

Nice.

So, the demo is still using non prefixed slide
For the deprecation tests, no need of two describes. Check how the others are done. One describe with one tests with logs suppressed and one with all errors.

@@ -413,3 +405,65 @@ function ($injector, $animate) {
}
};
}]);

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove 3 of the lines

Delete dfasfd�OM�OM
@wesleycho wesleycho closed this in 2e5bfac Oct 2, 2015
@wesleycho wesleycho added this to the 0.14.0 (Bootstrap 3.3) milestone Oct 2, 2015
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