Feat/st tabs default tab (new version)#11562
Feat/st tabs default tab (new version)#11562fridaeriksenaess wants to merge 5 commits intostreamlit:developfrom fridaeriksenaess:feat/st-tabs-default-tab
Conversation
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
kmcgrady
added
security-assessment-completed
change:feature
lukasmasuch
added
the
status:needs-product-approval
There was a problem hiding this comment.
Pull Request Overview
This pull request implements default tab selection behavior for the st.tabs element. Key changes include:
- Adding a default_tab_index field to the proto definition.
- Updating the Python API in layouts.py to support a new default parameter and validation.
- Adjusting the frontend Tabs.tsx component and adding new tests to verify default tab behavior.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| proto/streamlit/proto/Block.proto | Added default_tab_index field to TabContainer. |
| lib/tests/streamlit/elements/layouts_test.py | Added tests for default tab index behavior. |
| lib/streamlit/elements/layouts.py | Integrated default tab parameter and validation. |
| frontend/lib/src/components/elements/Tabs/Tabs.tsx | Updated state initialization and handling for tabs. |
| frontend/lib/src/components/elements/Tabs/Tabs.test.tsx | Added test case for default tab selection. |
Comments suppressed due to low confidence (1)
lib/streamlit/elements/layouts.py:528
- Duplicate return statement detected in the tab_proto function; the second return is unreachable. Please remove the redundant return statement to maintain clarity and avoid dead code.
return tuple(tab_container._block(tab_proto(tab_label)) for tab_label in tabs)
sfc-gh-bnisco
left a comment
sfc-gh-bnisco
left a comment
There was a problem hiding this comment.
Thanks @fridaeriksenaess for the contribution! I have a question inline about expected behavior. It also looks like the frontend typecheck is failing, could you please take a look at that? Let me know if you have questions, thank you!
|
|
||
| def test_duplicate_tab_labels(self): | ||
| """Test that duplicate tab labels are allowed.""" | ||
| tabs = ["Tab", "Tab", "Tab"] |
There was a problem hiding this comment.
question: What is the expected behavior if this is called with st.tabs(tabs, default="Tab")?
How about this scenario:
tabs = ["Tab A", "Tab B", "Tab B"]
st.tabs(tabs, default="Tab B")I suppose it could either raise an exception or take the first tab that matches the default param. @jrieke what do you think the behavior should be?
There was a problem hiding this comment.
Oh damn I didn't realize we allow duplicate tab names :D What do we do for st.multiselect in that case? Since that also allows duplicate options. Even though I guess it doesn't matter too much there because the options are equivalent if they have the same name (unlike tabs, which still represent different tabs if they have the same name).
Anyway, I'd lean towards selecting the first match. Raising an exception seems like overkill, especially since this also works for st.multiselect.
There was a problem hiding this comment.
+1 I think we should select the first match. I believe that's what is already happening in this implementation, but it would be great to get some tests to assert this behavior @fridaeriksenaess.
There was a problem hiding this comment.
What do we do for st.multiselect in that case?
Multiselect allows duplicate options, but - since the change to allow new options - it doesn't allow selecting duplicate options. A duplicated option is only selectable once. See #11219
jrieke
removed
the
status:needs-product-approval
|
When is this PR planned to merge, I'm in dire need of this feature. Our solution has a bug and can be fixed with this PR merge. |
lukasmasuch
added
the
status:product-approved
|
@fridaeriksenaess it looks like the remaining work for this PR is:
The python unit tests failure looks like it might just need a re-run. Are you able to work on these still? Otherwise, maybe I can finish this off so we can merge your PR. |
## Describe your changes - NOTE: This PR picks up the changes from #11562 and completes the remaining work items - Adds a `default` pram to `st.tabs` - Handles possible duplicate tab names ## GitHub Issue Link (if applicable) Fixes #10910 ## Testing Plan - ✅ Adds Unit Tests - JS and Python --- **Contribution License Agreement** By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license. --------- Co-authored-by: Frida Eriksen Næss <[email protected]>
Describe your changes
Messed up while rebasing PR #11267 so I made a new branch with the same changes.
GitHub Issue Link (if applicable)
#10910
Testing Plan
Added Python unit tests to verify that:
The correct tab index is set when using a label.
The correct tab index is set when using an index.
The default behavior is preserved when default is not provided.
Verified that the feature works via manual inspection of all_deltas.
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.