[Feature] Scrollable tabs #2861

fredriklindell · 2016-01-09T20:48:53Z

This commit implements request in below issue:

When device width is less than 992 tabs will paginate if the tab
container width is wider than device sreen width.

alitaheri · 2016-01-10T06:14:21Z

Wow very nice @fredriklindell 👍 👍 Thanks a lot. any chance you could also add an example in the docs site explicitly for this behavior?

oliviertassinari · 2016-01-10T13:54:32Z

@fredriklindell That looks like good start 👍.
Yes, could you add a new example of this so people know how to use it and to prevent regression.
Could you also fix the regression on the existing examples?
Before:

After:

Thanks!

fredriklindell · 2016-01-11T16:23:09Z

@oliviertassinari ofc i can fix a new example. Not sure what to add though, since the implementation works for above on small screens. I could add an example with more tabs - i'll do that :) what do you mean with regression, do you mean that the tab should stretch to fill the width? I could change to default prop for stretch to true, now it is false, should I do that?

fredriklindell · 2016-01-12T05:48:03Z

@oliviertassinari alright, thank you for the explaination. I changed the Tabs default prop to stretch by default. However doing so I noticed that the examples break. Due to that the widths of the html elements in componentDidMount is not correct. If a resize is triggered everything is working, it is just the initial state. No sure if it has to do with the CodeExample component. I have to investigate a bit more. Do you have any ideas? :)

I also added an example with 12 tabs.

oliviertassinari · 2016-01-12T18:31:58Z

src/tabs/tabs.jsx

  ],

  getDefaultProps() {
    return {
      initialSelectedIndex: 0,
+      stretchTabContainer: false,


should be true by default to not introduce breaking changes.
It could be changed later, with v0.15.0 if you think it's better this way.

oliviertassinari · 2016-01-12T18:34:48Z

I also added an example with 12 tabs.

That sounds like a good idea 👍.

fredriklindell · 2016-01-12T19:42:47Z

@oliviertassinari Correct, I have changed it to true.

But before I commit I need to fix the initial rendering error i am seeing in the examples. The widths read in componentDidMount is not correct. When I try this in my app it works. I have tried wrapping the code in my app with the ClearFix component - but it still works.

Any ideas? If you try by yourself you will notice that the ink-bar is not rendered correct. I think I need some help with this.

oliviertassinari · 2016-01-12T19:44:24Z

Any ideas?

I see that you have pushed the code, I will try to have a look.

fredriklindell · 2016-01-12T19:46:30Z

@oliviertassinari and regarding the possibility to change back to true in 0.15 sounds good, because the calculated width when stretching the tabs might not be equal to the width of the actual DOM element, thus making the ink-bar a bit offline to the tab.

fredriklindell · 2016-01-13T18:05:44Z

@oliviertassinari I have uploaded a new commit which somewhat fixes the problem. However, the strange thing is that the DOM elements are rendered with different widths between page reloads, it switched between two values with a rate of 50%. As I said I do not see these problems in my application only in the example page.

fredriklindell · 2016-01-15T05:45:18Z

@oliviertassinari have you had any time to look at the PR?

Thanks in advance.

vishalvijay · 2016-01-23T14:41:21Z

@fredriklindell Great work!!
@oliviertassinari When will this feature get released, waiting for it.

alitaheri · 2016-01-29T16:18:05Z

@fredriklindell We are very sorry it took so long. But there were a lot of things we had to take care of. There were a bunch of other features added to tabs so this needs rebasing. If you don't have the time to work on this, i can take it from here. Thanks for working on this 👍

fredriklindell · 2016-01-30T10:05:33Z

@oliviertassinari Any update on this PR? Do I need to compliment anything?

Thanks in advance!

fredriklindell · 2016-01-30T11:35:32Z

@alitaheri No worries, I will see what I can do tonight :)

oliviertassinari · 2016-01-30T12:01:44Z

@fredriklindell Sorry for not catching up earlier. I have various comments on this PR 👍

The existing examples shouldn't change from what is now and independently of the screen size.
Having a look at the specification here (https://www.google.com/design/spec/components/tabs.html#tabs-usage). I think that it would be good to add a scrollable boolean property. It would be false by default to not break the existing examples. When true, the width of the tab won't try to expand.
I feel like it's highly increasing the complexity of the Tabs and Tab components. I'm wondering if we can't move it to a new component. (That remember me this PR [AppBar] waterfall behaviour #1321).
The scrollable feature doesn't work on mobile devices, this don't have to be implemented in this PR, but I'm wondering how we would implement it once this PR is merged.
I think that it would be great to have this feature, I guess this can be done in another PR:

oliviertassinari · 2016-01-30T12:04:59Z

src/tabs/tabs.jsx

+  },
+
+  handleWindowWidthChange() {
+    this.replaceState(this.getNewState());


Can we not use replaceState?

This method is not available on ES6 class components that extend React.Component. It may be removed entirely in a future version of React.

This commit implements request in below issue: mui#1148 When device width is less than 992 tabs will paginate if the tab container width is wider than device sreen width.

Build and document error fixes

* New scrollable tab example added * Fix for (at least 50% of the time) for calculating widths in componentDidMount via requestAnimationFrame. But still the tab container width differs between reloads.

* Fixed for timing issue when components render, solves DOM element width problems.

* Rebase changes

* Changes according to comments

* Tabs does not consider device screen size anymore. Pagination will occur when tab container width is less than the total width of the tabs * Changed prop value stretchTabContainer to fillWidth Change-Id: Id63a096ab5d64ac90cccc000eab4b2811adfa043

vishalvijay · 2016-02-23T15:13:31Z

@oliviertassinari Will this PR get merged?

oliviertassinari · 2016-02-23T18:49:29Z

@vishalvijay We have agreed on splitting this PR into two chunks:

The first one is to implement a new propertyfillWidth. The default value would be false to keep the existing behavior.
The second one for implementing the overflow behavior.

vishalvijay · 2016-02-24T05:34:08Z

We have agreed on splitting this PR into two chunks

@oliviertassinari Somebody working on the same or I can give a try?

oliviertassinari · 2016-02-24T12:19:02Z

@vishalvijay Said that he is off for two weeks. If you can give it try, that would be awesome 👍.

alitaheri · 2016-03-10T20:20:38Z

I'm continuing this work. Nobody else pick this up please 😅

fredriklindell · 2016-03-12T12:12:40Z

Hi @alitaheri, when I left for vacation three weeks ago there where problem with the new tab implementation using enhanced button. I think that, if not already solved, we should have a look on that component and why the text is allowed to be wider that the button. If you ask me, the button size should adjust to the text width, just like a normal html button does. I highlighted this problem, but have not had the time to look intuit any further. Not sure what has happened in the main report since then, probably a lot :-)

alitaheri · 2016-03-12T12:39:47Z

@fredriklindell Yeah, a lot has changed. that's why I'm going to break down your PR into smaller ones and see if I can extract the logic into another component. This is a great feature 👍 I really need it in my app 😅 So I'll take it from here 👍

nathanmarks · 2016-03-15T02:51:42Z

@mbrookes This is PR: On Hold but is on the 0.15.0 release milestone, which isn't ideal 😄 What's the actual status?

alitaheri · 2016-03-15T05:21:58Z

@nathanmarks It's on hold because I'm working on it and this is just a place holder. I'm sure I can get it in the 0.15.0 release. 😁

nathanmarks · 2016-03-15T12:59:08Z

@alitaheri Ok, awesome! 👍

aviraldg · 2016-04-09T15:40:18Z

@alitaheri What's the progress on this? We need it in our app, so I wouldn't mind helping if you need it 😄

alitaheri · 2016-04-10T05:06:30Z

@aviraldg We do too, but I got a bit caught up with life, I plan on continuing the work on this in a couple of days. Although if you need this fast there are a couple of areas that should be fixed before this feature can be cleanly implemented.

First of all, The InkBar should not rely on the assumption that all tabs are of the same size, it should rely on the calculated width of each tab. After this, Tabs will need to be patched to work with the InkBar.

fillWidth for the Tabs should be implemented.

And then another component that wraps the Tabs should be implemented to provide the logic for this feature. it's not easy to migrate this after all the changes that happened. That's why it will probably need a re-implementation.

I would spend time to get this in. But recently I got too busy with life.

nathanmarks · 2016-05-07T15:32:27Z

@alitaheri Gonna finish this or want to close?

alitaheri · 2016-05-07T17:29:35Z

Man this was so hard to migrate after all the change that I couldn't even understand the old code. I gave up on it after I got busy. We still need this feature in our app though -_- I guess I'll have to implement it from scratch when we get there.

oliviertassinari added the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label Jan 10, 2016

oliviertassinari reviewed Jan 12, 2016
View reviewed changes

fredriklindell force-pushed the feature-scrollable-tabs branch from a084f7c to 7146329 Compare January 13, 2016 17:58

oliviertassinari mentioned this pull request Jan 14, 2016

[WIP][Core] Remove style-resizable mixin #2934

Closed

oliviertassinari reviewed Jan 30, 2016
View reviewed changes

fredriklindell added 6 commits January 30, 2016 22:01

[Feature] Scrollable tabs

c115455

This commit implements request in below issue: mui#1148 When device width is less than 992 tabs will paginate if the tab container width is wider than device sreen width.

[Feature] Scrollable tabs

80fd52f

This commit implements request in below issue: mui#1148 When device width is less than 992 tabs will paginate if the tab container width is wider than device sreen width.

Build and document error fixes

4a1e93c

Build and document error fixes

Render fix and new scollable example

2c521e0

* New scrollable tab example added * Fix for (at least 50% of the time) for calculating widths in componentDidMount via requestAnimationFrame. But still the tab container width differs between reloads.

Fixed for timing issue when components render

ef20dbd

* Fixed for timing issue when components render, solves DOM element width problems.

Rebase changes

a1e02e1

* Rebase changes

fredriklindell force-pushed the feature-scrollable-tabs branch from 120ba83 to a1e02e1 Compare January 30, 2016 21:23

Changes according to comments

17f948c

* Changes according to comments

Tab width changes

ea7b5cd

* Tabs does not consider device screen size anymore. Pagination will occur when tab container width is less than the total width of the tabs * Changed prop value stretchTabContainer to fillWidth Change-Id: Id63a096ab5d64ac90cccc000eab4b2811adfa043

mbrookes assigned oliviertassinari Mar 2, 2016

mbrookes added the new feature New feature or request label Mar 5, 2016

alitaheri mentioned this pull request Mar 10, 2016

[IconButton] Expose disableTouchRipple #3659

Merged

oliviertassinari added the on hold There is a blocker, we need to wait label Mar 12, 2016

mbrookes added this to the 0.15.0 Release milestone Mar 14, 2016

und3fined mentioned this pull request Mar 23, 2016

Init soure new tabs #3770

Closed

mbrookes assigned alitaheri and unassigned oliviertassinari Mar 23, 2016

oliviertassinari mentioned this pull request Mar 25, 2016

[Tabs] Default 'width' to 'auto' instead of '100 / n%' #3807

Closed

oliviertassinari modified the milestones: 0.16.0 Release, 0.15.0 Release Mar 29, 2016

alitaheri closed this May 7, 2016

mbrookes mentioned this pull request May 8, 2016

[Docs] Update the Roadmap #4205

Closed

und3fined mentioned this pull request May 20, 2016

[Tabs] Scrollable tabs #4308

Closed

oliviertassinari mentioned this pull request Oct 19, 2016

[Tabs] Let each tab set its own width #5415

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] Scrollable tabs #2861

[Feature] Scrollable tabs #2861

fredriklindell commented Jan 9, 2016

alitaheri commented Jan 10, 2016

oliviertassinari commented Jan 10, 2016

fredriklindell commented Jan 11, 2016

fredriklindell commented Jan 12, 2016

oliviertassinari Jan 12, 2016

oliviertassinari commented Jan 12, 2016

fredriklindell commented Jan 12, 2016

oliviertassinari commented Jan 12, 2016

fredriklindell commented Jan 12, 2016

fredriklindell commented Jan 13, 2016

fredriklindell commented Jan 15, 2016

vishalvijay commented Jan 23, 2016

alitaheri commented Jan 29, 2016

fredriklindell commented Jan 30, 2016

fredriklindell commented Jan 30, 2016

oliviertassinari commented Jan 30, 2016

oliviertassinari Jan 30, 2016

vishalvijay commented Feb 23, 2016

oliviertassinari commented Feb 23, 2016

vishalvijay commented Feb 24, 2016

oliviertassinari commented Feb 24, 2016

alitaheri commented Mar 10, 2016

fredriklindell commented Mar 12, 2016

alitaheri commented Mar 12, 2016

nathanmarks commented Mar 15, 2016

alitaheri commented Mar 15, 2016

nathanmarks commented Mar 15, 2016

aviraldg commented Apr 9, 2016

alitaheri commented Apr 10, 2016

nathanmarks commented May 7, 2016

alitaheri commented May 7, 2016

[Feature] Scrollable tabs #2861

[Feature] Scrollable tabs #2861

Conversation

fredriklindell commented Jan 9, 2016

alitaheri commented Jan 10, 2016

oliviertassinari commented Jan 10, 2016

fredriklindell commented Jan 11, 2016

fredriklindell commented Jan 12, 2016

oliviertassinari Jan 12, 2016

Choose a reason for hiding this comment

oliviertassinari commented Jan 12, 2016

fredriklindell commented Jan 12, 2016

oliviertassinari commented Jan 12, 2016

fredriklindell commented Jan 12, 2016

fredriklindell commented Jan 13, 2016

fredriklindell commented Jan 15, 2016

vishalvijay commented Jan 23, 2016

alitaheri commented Jan 29, 2016

fredriklindell commented Jan 30, 2016

fredriklindell commented Jan 30, 2016

oliviertassinari commented Jan 30, 2016

oliviertassinari Jan 30, 2016

Choose a reason for hiding this comment

vishalvijay commented Feb 23, 2016

oliviertassinari commented Feb 23, 2016

vishalvijay commented Feb 24, 2016

oliviertassinari commented Feb 24, 2016

alitaheri commented Mar 10, 2016

fredriklindell commented Mar 12, 2016

alitaheri commented Mar 12, 2016

nathanmarks commented Mar 15, 2016

alitaheri commented Mar 15, 2016

nathanmarks commented Mar 15, 2016

aviraldg commented Apr 9, 2016

alitaheri commented Apr 10, 2016

nathanmarks commented May 7, 2016

alitaheri commented May 7, 2016