Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WP components audit: TabPanel #16710

Closed
davewhitley opened this issue Jul 22, 2019 · 5 comments
Closed

WP components audit: TabPanel #16710

davewhitley opened this issue Jul 22, 2019 · 5 comments
Labels
[Feature] UI Components Impacts or related to the UI component system Needs Accessibility Feedback Need input from accessibility Needs Design Feedback Needs general design feedback. Needs Technical Feedback Needs testing from a developer perspective. [Package] Components /packages/components

Comments

@davewhitley
Copy link
Contributor

davewhitley commented Jul 22, 2019

Is your feature request related to a problem? Please describe.

This issue is part of a review on the naming, structure, and composition of the UI components as brought up in #16367

TabPanel

TabPanel

Audit

Opportunities I came across while reviewing this component.

  • Although, there is an aria role named tabpanel, This component has no obvious relation to the existing Panel component. If the Panel component is renamed this will be less of an issue.
  • It took me a long time to figure out that TabPanel also includes the content below as well (the "panels"). I thought TabPanel was just the row of tabs.
  • The vertical option for this component could be confusing, since that could be considered a Menu. There would be confusion on when to use one over the other. More concerns on vertical tabs.

Suggestions

After looking at a few libraries, I've come across a few different implementation strategies.

image

Option 1

<Tabs label="Colors">
     <Tab title="Red">
          Red tab content
     </Tab>
     <Tab title="Blue">
          Blue tab content
     </Tab>
     <Tab title="Yellow">
          Yellow tab content
     </Tab>
</Tabs>

Option 2

<Tabs tabs={tabs}>
     <Card>
          Tab content
     </Card>
</Tabs>

Option 3

<TabList>
     <Tab title="Red" />
     <Tab title="Blue" />
     <Tab title="Yellow" />
</TabList>
<TabPanel>
     {tabcontent}
</TabPanel>

Option 4 (example)

<Tabs>
     <Tab>Red</Tab>
     <Tab>Blue</Tab>
     <Tab>Yellow</Tab>
</Tabs>
<TabPanel>
     Red tab content
</TabPanel>
<TabPanel>
     Blue tab content
</TabPanel>
<TabPanel>
     Yellow tab content
</TabPanel>

To me, options 3 and 4 makes the most sense structurally, but I could see how option 2 could be convenient. Maybe we could have a mix of both?

This component can be broken down into several components:

  • The whole component: I like Tabs, but would also consider TabPanel.
  • The row of tabs: TabBar or TabList depending on implementation.
  • The tab itself: Tab
  • The content of the tab: TabPanel, Pane, or Card

Feedback

The feedback I'm looking for is related to naming, structure, and composition. I'm not looking for visual feedback of the components. Prop audit can be seen in the comment below.

I'm most interested in how you think the TabPanel component should be named and which composition is best.

@davewhitley davewhitley created this issue from a note in Component Audit (In Progress) Jul 22, 2019
@davewhitley
Copy link
Contributor Author

Props audit

As an addendum, here is a deeper evaluation of the props for these components. This is an effort to expand the components to be more useful and to answer the question, "Are properties well thought out and consistently applied?" This only covers visual props. Event handler props will be evaluated at a later date.

Overflow

To be useful, we need to consider how the tabs overflow and if we should provide different options in different contexts:

  • Automatically scroll left/right with a fade to indicate overflow. example
  • Show left/right arrow icon buttons for users to click and show hidden tabs. example
  • Show an overflow icon button at the end and show additional options on click/tap. example

✨ New

  • centered*: displays the tabs as centered
  • fullWidth*: 100% width of container. Each tab is a % width of the container
  • icon*: show an icon with the label of the tab
  • stacked*: Stacks the icon on top of the text label

*not necessary, but would greatly enhance the usefulness of the component.

✂️ Remove

  • orientation: see audit in original comment above

Event handler props will be evaluated at a later date.

@davewhitley davewhitley added [Package] Components /packages/components Needs Design Feedback Needs general design feedback. Needs Technical Feedback Needs testing from a developer perspective. [Feature] UI Components Impacts or related to the UI component system labels Jul 22, 2019
@davewhitley davewhitley moved this from In Progress to Needs feedback in Component Audit Jul 22, 2019
@jffng
Copy link
Contributor

jffng commented Jul 23, 2019

Per #13587, it's worth noting that the TabPanel component is not used anywhere in the codebase yet. So it could be a good opportunity to get the desired props in that we want now, maybe as a part of #16663

@davewhitley
Copy link
Contributor Author

Further explanation on removing the vertical orientation:

Vertical orientation is a common feature of tabs in component libraries and it's an ARIA widget, but I believe this could be an outdated way of displaying tabs.

Some concerns:

  • Doesn't consider mobile very well. It's very desktop-first. In small viewports, the tabs would need to collapse into a horizontal row, which is the default anyway. Vertical tabs discourage thinking about mobile.
  • It blurs the line between a sidebar menu and tabs, which are not meant for navigation.
  • It might encourage people to nest tabs, which we don't want to do (a horizontal row of tabs inside of a vertical tab panel).
  • I can't rationalize the need for a vertical tab layout, unless it's to make better use of space on a wide screen.

This may be why some mature component libraries don't have this option, like Material and Polaris.

@barraponto
Copy link

Tab contents can't link to other tabs -- they have no way to control tab state. It could be fixed by passing an extra parameter to the children function, like this:

this.props.children(selectedTab, this.handleClick)

Don't know if this is the proper place to ask for new props or enhanced props (but it seemed like it).

@mapk
Copy link
Contributor

mapk commented May 12, 2020

I'm closing out the Component Audit issues for now.

@mapk mapk closed this as completed May 12, 2020
Component Audit automation moved this from Needs feedback to Done May 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] UI Components Impacts or related to the UI component system Needs Accessibility Feedback Need input from accessibility Needs Design Feedback Needs general design feedback. Needs Technical Feedback Needs testing from a developer perspective. [Package] Components /packages/components
Projects
No open projects
Development

No branches or pull requests

4 participants