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

fix(tabs): added controllerAs for tab controller #5020

Closed
wants to merge 1 commit into from
Closed

fix(tabs): added controllerAs for tab controller #5020

wants to merge 1 commit into from

Conversation

stalniy
Copy link

@stalniy stalniy commented Dec 6, 2015

it fixes #5019

@wesleycho
Copy link
Contributor

This needs tests.

@stalniy
Copy link
Author

stalniy commented Dec 6, 2015

@wesleycho Done

@@ -98,6 +98,10 @@ describe('tabs', function() {
titles().eq(1).find('> div').click();
expect(scope.deselectFirst).toHaveBeenCalled();
});

it('should expose controller as "tab" scope variable', function() {
expect(typeof elm.find('.uib-tab').first().isolateScope().tab).toEqual('object');
Copy link
Contributor

Choose a reason for hiding this comment

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

Using a typeof check is not great, and this test is a little too vague. It would be better to test against what it equals exactly.

Copy link
Author

Choose a reason for hiding this comment

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

I believe the only way to do this is:

it(..., inject(function(uibtabDirective) {
  expect(elm.find('.uib-tab').first().isolateScope().tab).toBeInstanceOf(uibtabDirective[0].controller);
}));

Let me know if you are ok with such solution

Copy link
Contributor

Choose a reason for hiding this comment

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

Check how some of the other tests for the other components is testing for controllerAs working - those are more useful guides as far as a readable test for this feature are written.

Copy link
Author

Choose a reason for hiding this comment

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

@wesleycho Ok, thanks for pointing me to the right direction. Done

@wesleycho wesleycho added this to the 1.0.0 milestone Dec 7, 2015
@wesleycho wesleycho closed this in a5cac90 Dec 7, 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.

Unable to extend tab's template
2 participants