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

Tabs validate parent is proper tab container #2463

Merged
merged 7 commits into from Jan 30, 2024

Conversation

tankztz
Copy link
Contributor

@tankztz tankztz commented Jan 26, 2024

All Submissions:

  • Have you followed the guidelines stated in CONTRIBUTING.md file?
  • Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

After these steps, you're ready to open a pull request.

a. Give a descriptive title to your PR.

b. Describe your changes.

c. Put `closes #XXXX` in your comment to auto-close the issue that your PR fixes (if such).

@tankztz
Copy link
Contributor Author

tankztz commented Jan 26, 2024

closes #2173

Copy link
Collaborator

@masenf masenf left a comment

Choose a reason for hiding this comment

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

is there a reason why using _valid_children didn't work for this case?

@tankztz
Copy link
Contributor Author

tankztz commented Jan 26, 2024

is there a reason why using _valid_children didn't work for this case?

The requirement here is: when user create a component of rx.SomeComponent(rx.TabList) we need to report error.

_valid_children means "only components that are allowed as children". Adding TabList as _valid_children for Tabs doesn't fix the bug because the parent of TabList could be anything.

Here are two ideas:

  1. Declare TabList as invalid child for all component except Tabs - it is changing too many things
  2. Declare the parent of TabList should only be Tabs - This is what I'm doing

@masenf
Copy link
Collaborator

masenf commented Jan 26, 2024

What about making TabList and TabPanels and _valid_children of Tabs though? Just taking everything up a level.

Okay wait, that's what you already said... let me think about it a bit more.

@tankztz
Copy link
Contributor Author

tankztz commented Jan 29, 2024

@masenf Any follow-ups?

Copy link
Contributor

@picklelo picklelo left a comment

Choose a reason for hiding this comment

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

nice addition - this will be useful for our new radix components as well which have nested components like this.

@picklelo picklelo merged commit 209c5fa into reflex-dev:main Jan 30, 2024
44 of 45 checks passed
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 this pull request may close these issues.

None yet

3 participants