add paper-tabs component

[panthony]

Add paper-tabs component.

This PR is based on #540 with:

• rebased on the current master
• inline font reverted
• usage of ember-composability-tools addon
• bunch of fixes

This is not 100% perfect but I think it could do the job for a first version.

[miguelcobain]

This will probably lead to jscs errors since Ember. access is forbidden.

[panthony]

Oh yes, the new disallowDirectPropertyAccess directive set to true by default now I believe.

Will update the code accordingly.

[miguelcobain]

This is now invalid notation. We should use tabs.@each.id instead.

[panthony]

Will fix that too, thanks.

I did not really reviewed all the code, just made sure the demo was working

[panthony]

Urgh, unfortunately it still quite rather buggy.

It sets a width for each tab in the style attribute but not properly all the time:

[mellatone]

@panthony What you can do for tests that need a wormhole, is to add <div id="paper-wormhole"></div> before the component in the hbs block.

[panthony]

@mellatone Thanks for the tips, not sure when I'll have time to write proper tests though.

[wooandoo]

Hello,

I imported the paper-tabs components in my project whose version is 2.10 (with glimmer).

I had the error:

"Assertion Failed: You modified wormhole twice on <izi-done-ui@component:paper-tab-body::ember843> in a single render. This was unreliable and slow in Ember 1.x and is no longer supported. See emberjs/ember.js#13948 for more details."

The reason is due to the issue:
emberjs/ember.js#13948

I fixed it in paper-tabs-content-wrapper:

init() {
    this._super();
    Ember.run.scheduleOnce('afterRender', () => {
      this.get('parent').send('setWormhole', this.elementId);
    })
    // this.get('parent').send('setWormhole', this.elementId);
  }

the tabs are now visible and I can swap between them.

I updated the SCSS to see the ink bar:

md-ink-bar {
  background-color: red;
}

paper-tabs will be very usefull. ;-)

[panthony]

This callback is never removed from the window object, it keeps get called even after the object has been destroyed.

[miguelcobain]

Also, we should namespace the event binding. That will allow us to later remove just the binding that was added. This makes it possible to have multiple instances of tabs (and possibly other components that rely on the resize event) without any colisions.

More info: https://css-tricks.com/namespaced-events-jquery/

[bjornharrtell]

Tried this branch out and it looks really nice. I'm willing to work on the remaining issues.

[panthony]

@bjornharrtell If you can it would be with pleasure, for now it's doing the job so I can't really allocate more time to it.

[bjornharrtell]

Might very well be working well enough for me too (thanks @mellatone and @panthony for the hard work!), but would be nice to have it in a future proper release.

[bjornharrtell]

Great stuff @panthony. Added examples are great to have and they demonstrate the tab title size issue more clearly than before. Might be that if we can fix that it's good enough to merge... but what are your thoughts on refactor to use ember-composability-tools?

[panthony]

@bjornharrtell Did not take a look to ember-composability-tools yet and I'm not sure I'll have the time.

I'm currently trying to write some tests on the prev/next actions but i do not always have the same width for the pagination wrapper. And some widths are computed multiple times (with same results) during first render.

Maybe related to the truncated label issue as it does not seems to be consistent.

Also I wanted to abstract/DRY a little the tabs offsets and widths handling with volatile properties, ex:

  domObject: computed('self.id', function() {
    return this.$(this.get('self.id'))[0];
  }),

  pagingWith: computed('domObject', function() {
    let domObject = this.get('domObject');
    return domObject.offsetLeft + domObject.clientWidth;
  }).volatile(),

I found another issue with dynamicHeight, it does not update if the content is updated but also if you put a paper-tabs with dynamic height in a tab (also with dynamic height) the whole content will be clipped.

May be acceptable for a first release though, probably an edge case.

Lastly I'd like to fix the stretched tabs handling, the ink bar is buggy.

Edit:

I'll probably settle after fixing the stretch and label issue because it lacks tests before refactoring the sizes handling. But tests are hard to write when result is inconsistent :(

[panthony]

@bjornharrtell To fix the tab's width issue I tried to specify the tab's width only if they should be stretched, in this case, the tab's width should be canvas' width / tabs.length.

But I still have the issue of the ink bar positioning when tabs are stretched.

They are based on pagingWidth which is not properly computed when stretched and I wanted to return the canvasWidth (since tabs takes all the width and only the width) but it makes a circular reference because the shouldStretchTabs already depends on canvasWidth (to determine if true/false when auto ).

I'd like to find a way to implement the auto option without checking the canvasWidth...

[bjornharrtell]

Agreed, this is tricky stuff and I guess it might be susceptible to cross browser issues. I'm still not into the code enough to help solving it in the short term. :(

[panthony]

@miguelcobain I'm finally done with the rework of the paper-tabs component.

There are probably corner cases here and there (dynamic content in tabs with dynamic height, etc.) but I think for a first version it's better than nothing.

Let me know if you think we can merge this fairly soon since I'm gonna need this.

[panthony]

Great, passing tests in local fails in travis ❤️

[bjornharrtell]

Nice work @panthony. Tried it out locally and it looks great. I'm still interested in using this component.

I get two test failures locally that seem fairly trivial but on Travis CI things seems weird. I'll report back when I've been able to test some more.

[panthony]

@bjornharrtell I may have out of date local dependencies in that case, or maybe a different version of phantomjs.

But right now I must take care of something else for work so if you know how to fix some of the failing tests I'd be grateful for any inputs 👍

[bjornharrtell]

The two failing cases I got are definitely PhantomJS related. All pass in Chrome.

The first failing case works when adjusting style assertions:

@@ -339,11 +339,11 @@ test('it renders ink bar properly', function(assert) {
 
   this.$('md-tab-item:nth(2)').click();
 
-  assert.equal(this.$('md-ink-bar').attr('style'), 'left:188px;right:0px;');
+  assert.equal(this.$('md-ink-bar').attr('style'), 'left:187px;right:0px;');
 
   this.$('md-tab-item:nth(1)').click();
 
-  assert.equal(this.$('md-ink-bar').attr('style'), 'left:85px;right:88px;');
+  assert.equal(this.$('md-ink-bar').attr('style'), 'left:85px;right:89px;');
 
   this.$('md-tab-item:nth(0)').click();

Not sure on what to do for a real fix though.

The second failing case is on a Next button is enabled assertion in the it can paginate tabs test. I suspect there is a timing issue regarding the style transformation but haven't come up with a workaround.

[panthony]

That's sad for the first style assertions. I guess if you fix on phantomjs you break it in chrome ?

Maybe this is a rounding issue somewhere that could be fixed by ceiling the values :/

[bjornharrtell]

About the Travis CI failures it seems ember try:one ember-release fails badly (38 failing cases) for me locally too on a fresh checkout of the master branch. Also noted that the exact same cases fail when running ember try:one ember-lts-2.8.

[bjornharrtell]

I think the Travis CI problems will be fixed by #694.

[panthony]

Cool!

As soon as I have some time I'll reinstall my local dependencies, bump phantomjs if necessary, downgrade the qunitjs version locally and re-take a look at the remaining failing tests.

[bjornharrtell]

Should be a matter of rebasing now. If you add me as a collaborator on your fork I'll do it for you. :)

[panthony]

Thanks @bjornharrtell but I can't add a collaborator.

I commented out both "it can paginate" and "it renders ink bar properly" as I don't have any easy solution at the moment.

[panthony]

Fails with "ember-beta" with "expected an implementation for 6".

No clue what is that. Google is not helpful.

[panthony]

@miguelcobain Done, but now I have the same issue than I had on the separate addon I did for the tabs.

There is a bug of display with the ink bar on the first load (apparently it works if resources are cached by the browser).

The mesured width for md-pagination-wrapper in the afterRender loop is not the same as the final rendered width.

I'm not really how to fix this.

We "need" to mesure the width of md-pagination-wrapper to check if it's greater than the canvas width, if so, the pagination is enabled.

[panthony]

Urgh the new styling is breaking some cases.

Like now if tabs are centerer the canvas has overflow:hidden which negate the check if the tabs should be paginated or not.

Need to review the precedence between:

• should the tabs be stretched or not
• should the tabs be paginated or not
• should the tabs be centered or not

My guess is that "should paginate" has precedence because if they do not fit in the canvas there is no point really to center or stretch them.

Edit: it doesn't even fix the problem because during render of md-tabs the pagingWidth is not yet computed so we assume we want to center them, and because of that when we try to measure the pagingWidth we end up with the same width than the canvas.

I really can't spent a day or two trying to figure out how to fix this so help would be really appreciated.

[Luiz-N]

+1

[panthony]

closing in favor of #755