Add tabs and container components

[bytasv]

• Closes Support Tabs component #747

CleanShot.2023-01-12.at.14.37.45.mp4

@prakhargupta1 I know it's not a "tabbed container" that was discussed originally in the task, but IMO this comes closer to what we want to achieve right now.

If there won't be objections I would propose to continue polishing the approach as follow up as I realised to do something as a tabbed container we would probably need to do major work on how things are stored in our data structure.

This PR introduces 2 components:

• Tabs
• Container which has "visible" control

By using Tabs UI we can control visibility of container.

Now to actually do a "tabbed container" which would automatically swap items, I'll probably need some guidance either from @apedroferreira or from @Janpot. If any of you guys have any tips before going that road - how should we handle adding components to a container and switching active container within same component? (asking it here, as I'm not sure if we will agree on a current implementation as a intermediate solution or should that be replaced with more advanced implementation)

[apedroferreira]

Thanks for this effort, seems to work well!

I'm not sure if I like the approach with the visible prop, even though it works.
It seems like the only reason we are introducing the visible prop in the new Container component is for this use case.
I would prefer if we were able to make it work like you mentioned, even though it's more challenging:

to actually do a "tabbed container" which would automatically swap items

I think this alternative sounds a lot better and more intuitive.
But we would probably need a new type of element prop that can take a list of children, as opposed to just one (elementList maybe?). The same thing as the children prop in the Paper component, for example, but with a list of views instead of a single view.

Then in the Tabs component logic we could make it render the correct view according to the active tab.

This might require special logic in the render overlay or the page view state so that when we dropped an element in an elementList slot, this element is added to the correct tab... I'd have to look into it deeper to see if there would be many more obstacles, or if this would be a good way to do things, but this is at least where I would start.

[bytasv]

I'm not sure if I like the approach with the visible prop, even though it works.
It seems like the only reason we are introducing the visible prop in the new Container component is for this use case.

At the moment yes, there is nothing else that could use this prop, however I think controlling visibility is something that we currently lack in general and it should be useful. I.e. if we built a form with checkbox/radio and expected that some content would appear depending on selection, this component could also be used. Unless we decide on some different method to control visibility of components

[apedroferreira]

At the moment yes, there is nothing else that could use this prop, however I think controlling visibility is something that we currently lack in general and it should be useful. I.e. if we built a form with checkbox/radio and expected that some content would appear depending on selection, this component could also be used. Unless we decide on some different method to control visibility of components

Yeah that's a good point. It sounds fine to me to keep the prop in the Container component, at least it sounds better than having this prop in every component... Unless it can be controlled with the sx prop?

[apedroferreira]

Another thing: there is a bug I've found so if this prop is not called children it doesn't work, but it should. I'm including the fix in #1527

[prakhargupta1]

Interesting! I think for now this looks good to me.
QQ: Is it possible for this container component to have sx prop? The Reason I am asking it is, I tried previewing tabs, and they appear like shown below, if there was an option to add border to a container then I could entire page in a container (like paper but without any elevation)

[bytasv]

QQ: Is it possible for this container component to have sx prop?

@prakhargupta1 yes ofc, I can add it. I just wanted to share this option quickly, but if we are fine with intermediate solution then I can improve it as much as needed and work on tabbed container improvements in follow up

[Janpot]

We will need to expand on the 'object' argtype, to include its properties. That way we can resolve it recursively in the runtime and find and render nested children and replace them in the object before we pass it as props to the component.

export interface ObjectValueType extends ValueTypeBase {
  type: 'object';
  /** @deprecated lets remove this: */
  schema?: string;
  properties?: Record<string, PropValueType>
}

export interface ArrayValueType extends ValueTypeBase {
  type: 'array';
  /** @deprecated lets remove this: */
  schema?: string;
  items?: PropValueType
}

that way we can define something like:

argTypes: {
  tabs: {
    typedef: { 
      type: 'array',
      items: {
        type: 'object',
        properties: {
          label: { type: string },
          content: { type: 'element' }
        }
      }
    }
  }
}

Not 100% sure this will be enough to solve the tabs problem. To be further investigated.

I think this should happen as a separate effort from adding new components

[bytasv]

@apedroferreira added sx prop with defaults for padding and border as @prakhargupta1 requested

[apedroferreira]

Looks good to me as a first approach!

Like I said maybe we could consider also controlling visibility with the sx prop instead of having a prop just for it in the Containers, but I'll leave that up to you, as we can also improve this with the more difficult solution later.

[Janpot]

This should be value

[bytasv]

Can you clarify what you mean? the name of prop? if yes, why?

[Janpot]

the name of prop? if yes, why?

yes, because:

1. Conventions: we generally use the combination value/onChange to denote the main controlled prop of a component (yes, there are specific situations where we deviate from this, e.g. DataGrid.)
2. The MUI Tabs component uses value
3. This component as it currently is, is in essence a Select, and the MUI Select uses value
4. The word "active" is an adjective, and "a tab" is "a thing", so we should use a noun for naming it in our code.

[Janpot]

I'm not sure about the tabs. It feels like a Select to me with a different UI. I'm not sure how the migration path from this towards a real tabs component would work. Would it make more sense to have "tabs" as an option on the Select component? That way we keep the Tabs available for implementing real tabs, with surface one can build UI in?

One thing to note is that the container allows users creating flows that cause vertical layout shift, which is something we have avoided up until now.

[bytasv]

I'm not sure about the tabs. It feels like a Select to me with a different UI. I'm not sure how the migration path from this towards a real tabs component would work. Would it make more sense to have "tabs" as an option on the Select component? That way we keep the Tabs available for implementing real tabs, with surface one can build UI in?

retool has both Tabs and TabbedContainer which I guess would be "real tabs" as you call it. IMO having Tabs placed under Select would significantly impact it's discoverability.

One thing to note is that the container allows users creating flows that cause vertical layout shift, which is something we have avoided up until now.

Avoided intentionally? Or is this just part of a change with new features? Also if ensuring no layout shifts happens is so important, users can use containers with fixed height via sx prop I assume.

[Janpot]

Retool ended up with a lot of components that kinda do the same, but different. And I'm not sure that it's such a good thing. Why would you need Tabs if you have Tabbed container? The latter will do for 99% of the use-cases. And former has an equal status as a component for its niche use-case. IMO it would make more sense to have a set of core components that do 99% of the work and put all the niche ones further away.

But now that I think of it, we can always migrate the current Tabs as an option under Select when the time comes. So nevermind.

Avoided intentionally?

It was avoided intentionally. The main idea was that by default, when it comes to building good UX, users should fall in a pit of success. Making it hard to build layout shifting UI was part of that. But users were always able to circumvent with the sx prop, so I'm not saying this should be a hard rule. Just sharing design considerations that were adhered to up until now.