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.

Sign up for GitHub

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

Jump to bottom

add usePanel prop for tabs component #392

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

add usePanel prop for tabs component #392

wants to merge 1 commit into from

Conversation

ozanturhan
Copy link

  • 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

closes #329

add usePanel prop for tabs component
322600a 
- 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
@rdeveen
Copy link

rdeveen commented May 11, 2021

This would be nice.

@RachelElysia RachelElysia mentioned this pull request Jun 4, 2021
Open
@rdeveen
Copy link

rdeveen commented Nov 12, 2021

@danez can this be released?

@bencrabtree
Copy link

@ozanturhan Is there an ETA when this will be released?

@ozanturhan
Copy link
Author

@ozanturhan Is there an ETA when this will be released?

@bencrabtree I don't have any idea about it. I'm still waiting :)

@bencrabtree
Copy link

@ozanturhan Thanks for the quick response! I was curious if I could request this patch to be prioritized. It seems as though it's all ready to go, relatively low stakes, and my team would greatly appreciate it!

@joepvl
Copy link
Collaborator

joepvl commented Feb 4, 2022 •
edited
Loading

I don't see the use case for this. Without panels, what's this component but a list of items?
In addition, if I'm reading it correctly the implementation in this PR would also enable consumers to provide fewer or more panels than tabs when passing usePanel={true}, which is undesirable. If you have a concrete problem you're trying to solve with this PR, please open an issue.

@ozanturhan
Copy link
Author

I don't see the use case for this. Without panels, what's this component but a list of items?
In addition, if I'm reading it correctly the implementation in this PR would also enable consumers to provide fewer or more panels than tabs when passing usePanel={true}, which is undesirable. If you have a concrete problem you're trying to solve with this PR, please open an issue.

There is an issue actually. I mentioned it in my first comment in this pull request. Here is the issue: #329

@joepvl
Copy link
Collaborator

joepvl commented Feb 4, 2022

Apologies, I missed that! Thanks.

@ozanturhan
Copy link
Author

@joepvl No problem. I can also create an example when I find time for it. Also if my implementation don't fit your expectation we can discuss it and find proper solution.

hg-pyun
hg-pyun reviewed Mar 8, 2022
View reviewed changes
Copy link
Member

@hg-pyun hg-pyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm

AbdullahWins
AbdullahWins approved these changes Aug 22, 2023
View reviewed changes
Copy link

@AbdullahWins AbdullahWins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good Job!

AbdullahWins
AbdullahWins approved these changes Aug 24, 2023
View reviewed changes
Copy link

@AbdullahWins AbdullahWins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems Alright!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hg-pyun hg-pyun hg-pyun left review comments

@suresh4sparktics suresh4sparktics suresh4sparktics approved these changes

@jocarrd jocarrd jocarrd approved these changes

@AbdullahWins AbdullahWins AbdullahWins approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Disable warning for TabPanel count of 0
9 participants
@ozanturhan @rdeveen @bencrabtree @joepvl @hg-pyun @AbdullahWins @jocarrd @suresh4sparktics @danez