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

Disable warning for TabPanel count of 0 #329

Open
spectras opened this issue May 28, 2020 · 2 comments · May be fixed by #392
Open

Disable warning for TabPanel count of 0 #329

spectras opened this issue May 28, 2020 · 2 comments · May be fixed by #392

Comments

@spectras
Copy link

Hello,

I am using react-tabs with with success in my app.
In some places though, I use it with no panel at all. In that case, it is used to change a property on another component.

Stripped down example to illustrate:

const [page, setPage] = useState(0);

<Tabs selectedIndex={page} onSelect={setPage}>
    <TabList>{ /* one Tab per page here */ }</TabList>
</Tab>
<DocumentView url={url} page={page} />

This works perfectly.

However, react-tabs complains that

There should be an equal number of 'Tab' and 'TabPanel' in Tabs

It then continues happily and everything works fine.

  • Do you see any issue with this use case?
  • If not, would it be possible to silence that warning when the number of TabPanels is exactly 0?
ozanturhan pushed a commit to ozanturhan/react-tabs that referenced this issue May 7, 2021
- With this commit, tabs can be used without panels.
- If usePanel is given false, the error that occurs when there are no panels is not printed in console.
- usePanel default value is true.
- this commit does not cause any breaking change

reactjs#329
ozanturhan pushed a commit to ozanturhan/react-tabs that referenced this issue May 7, 2021
- With this commit, tabs can be used without panels.
- If usePanel is given false, the error that occurs when there are no panels is not printed in console.
- usePanel default value is true.
- this commit does not cause any breaking change

reactjs#329
@ozanturhan ozanturhan linked a pull request May 7, 2021 that will close this issue
@joepvl
Copy link
Collaborator

joepvl commented Mar 10, 2022

This would add more surface to the API, only to support a use case that was obviously not intended to be supported. (And in my personal opinion does not make sense.)

As an alternative, maybe we should allow the use of TabList outside of Tabs instead? Perhaps export a standalone variant of TabList that takes selectedIndex and onSelect props.

Although, to be honest, I don't see a meaningful advantage over adding your desired behaviour to <ul><li></li></ul>, akin to my comment in the fleetdm/fleet issue that references this issue.

@spectras
Copy link
Author

It sure would add more surface to the API, so the concern of whether it adds value makes sense.

The value in my case - and also it seems on the issue you linked - does not come from instantiating a bunch of <ul> and <li>s but from the user interactions.

  • Handling of arrow keys.
  • Preventing the tab bar from jumping in some cases.
  • Dealing with RTL text subtleties.
  • Possibly ARIA stuff - I did not check whether react-tabs does those.

If all the UX logic was exported as a set of react hooks, then yes I would have a bunch of <ul> and <li>s and apply UX logic through hooks. That would indeed be much better from a design perspective.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants