fix(tabs): remove inline css width if tabs should stretch

[JSMike]

There is an issue when dynamically setting md-stretch-tabs. If shouldStretchTabs() evaluates to false at any time then a fixed width is added and never removed. This fix removes a fixed inline width from <md-pagination-wrapper> if shoulStretchTabs() returns true.

[googlebot]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

[googlebot]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

[googlebot]

CLAs look good, thanks!

[googlebot]

CLAs look good, thanks!

[JSMike]

Updated my local git e-mail (was using my .edu)
Fixed commit message to include component.

[ThomasBurleson]

@JSMike - Seems like this fix introduces a perf/reflow issue?

[ThomasBurleson]

@topherfangio - review ?

[topherfangio]

@JSMike I think this would be better if we moved this logic (including the if) into the updatePagingWidth() method since it also sets the width of the elements.paging element.

Ideally some comments and a test would be great too.

[JSMike]

Ok I'll take a look.

[JSMike]

I'm not quite sure why tests are failing in the utils test suite, and only when minified.

@topherfangio Could you let me know if the changes I made were in line with what you were thinking?

[JSMike]

Just confirmed it was due to my adding $timeout.flush() to the tabs tests to make sure all the init() functions run. I'll see if there's a better way.

[JSMike]

I removed the call to $timeout.flush() and instead used a spy on $mdUtil.nextTick to control when the functions were called. I haven't use jasmine much so I'm not sure if this was the most elegant solution, let me know if you'd prefer something else.

[topherfangio]

@JSMike Out of curiosity: did you try adding the $timeout.flush() inside of your tests after you made the setup() call? I can see how adding it directly inside the setup method might cause the other tests to fail, but just putting it in your own test would be a lot cleaner than putting the spy on $mdUtil.nextTick().

Two other notes:

1. If you append an element to the DOM in your test, you should make sure to call element.remove() at the end of the test so that it doesn't affect other tests. It might be best if you either moved the append into your tests, or added an afterEach() that checked to see if something was appended and then removed it.

2. I generally prefer to put the inject() statement as part of the it() clause like below so that it wraps the entire test:

   it('does something cool', inject(function($timeout) {
       // all test code goes here
   }));

Thanks so much for tackling this! Please let me know if you have any questions or need any guidance, I am happy to help and even do a quick call if needed.

[JSMike]

@topherfangio You're correct with $timeout.flush() inside of the setup causing other tests to fail. The pattern with inject wrapping the entire test function makes sense, I didn't realize that some tests were already doing that and copied one that just used an anon function, that's why I was having issues. I'll look into this some more tonight.

Does everything look better in tabsController.js as far as cleaning up the functions?

The only other question I have for right now, should I rebase this PR into one commit?

[topherfangio]

@JSMike Yes, everything in the tabsController.js looks great. I don't see any needed changes there, just make sure to rebase against the latest master branch as I think there may be changes.

If you can squash it into a single commit, that would be great!

Thanks again!

[JSMike]

Fixed tests and rebased.

Note: I was working on tests to set an initial inline style, update md-stretch-tabs, and confirm inline style change. I was having success in chrome but I couldn't get it to work in phantom so I took them out.

[topherfangio]

@ThomasBurleson LGTM

[ThomasBurleson]

@JSMike - Kudos!