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

fix(carousel): re-enable deprecated directives #4527

Closed
wants to merge 1 commit into from

Conversation

wesleycho
Copy link
Contributor

  • Re-implement deprecated directives
  • Re-expose CarouselController with deprecation notice

expect($log.warn.calls.count()).toBe(2);
expect($log.warn.calls.argsFor(0)).toEqual(['slide is now deprecated. Use uib-slide instead.']);
expect($log.warn.calls.count()).toBe(3);
expect($log.warn.calls.argsFor(1)).toEqual(['slide is now deprecated. Use uib-slide instead.']);
Copy link
Contributor

Choose a reason for hiding this comment

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

Check all messages.

@wesleycho wesleycho force-pushed the fix/carousel-rename branch from 2fd509c to 86ed220 Compare October 3, 2015 21:04
@wesleycho
Copy link
Contributor Author

Added other checks as noted

@Foxandxss
Copy link
Contributor

👍

@chrisirhc
Copy link
Contributor

Just wondering why this PR was created. Is it to resolve some issues? It should reference those issues, if that's the case, to track the reason for the change.

@Foxandxss
Copy link
Contributor

@chrisirhc We are prefixing all the directives. There were a slip, and in the carousel, the deprecated directives were empty. They need to be fully functional. This PR puts back the functionality on those deprecated directives.

This shouldn't be a fix(foo) tho but chore.

$log.warn('CarouselController is now deprecated. Use UibCarouselController instead.');
}

angular.extend(this, $controller('UibCarouselController', {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of extending this, you can probably return the $controller('Uib… directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Foxandxss mind if I just patch this in all of the components?

- Re-implement deprecated directives
- Re-expose `CarouselController` with deprecation notice
@wesleycho wesleycho force-pushed the fix/carousel-rename branch from 86ed220 to 3c8f22e Compare October 4, 2015 00:31
@wesleycho wesleycho closed this in 30e8aa7 Oct 5, 2015
@wesleycho wesleycho deleted the fix/carousel-rename branch October 5, 2015 00:06
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