Skip to content

Conversation

@sfc-gh-bnisco
Copy link
Collaborator

@sfc-gh-bnisco sfc-gh-bnisco commented Dec 4, 2025

Describe your changes

  • Fixes a regression where both the Sidebar and Top nav would be visible when navigation is set to top.

GitHub Issue Link (if applicable)

Fixes #13224

Testing Plan

  • Adds e2e tests

Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

@snyk-io
Copy link
Contributor

snyk-io bot commented Dec 4, 2025

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Copy link
Collaborator Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2025

✅ PR preview is ready!

Name Link
📦 Wheel file https://core-previews.s3-us-west-2.amazonaws.com/pr-13227/streamlit-1.52.0-py3-none-any.whl
📦 @streamlit/component-v2-lib Download from artifacts
🕹️ Preview app pr-13227.streamlit.app (☁️ Deploy here if not accessible)

@sfc-gh-bnisco sfc-gh-bnisco added security-assessment-completed Security assessment has been completed for PR change:bugfix PR contains bug fix implementation impact:users PR changes affect end users feature:st.navigation Related to `st.navigation` labels Dec 4, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2025

📉 Frontend coverage change detected

The frontend unit test (vitest) coverage has decreased by 0.0500%

  • Current PR: 85.8500% (12413 lines, 1756 missed)
  • Latest develop: 85.9000% (12413 lines, 1750 missed)

💡 Consider adding more unit tests to maintain or improve coverage.

📊 View detailed coverage comparison

@sfc-gh-bnisco sfc-gh-bnisco marked this pull request as ready for review December 5, 2025 05:12
Copilot AI review requested due to automatic review settings December 5, 2025 05:12
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a regression where both the sidebar navigation and top navigation were simultaneously visible when the navigation position was set to "top". The fix ensures that when top navigation is enabled, the sidebar navigation is properly hidden.

Key Changes

  • Frontend logic now correctly hides sidebar navigation when top navigation position is active
  • Comprehensive E2E tests added to verify all three navigation positions (sidebar, top, hidden)

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
frontend/app/src/App.tsx Adds condition to hide sidebar navigation when effectiveNavigationPosition === Navigation.Position.TOP
e2e_playwright/st_navigation_test.py New E2E tests verifying sidebar, top, and hidden navigation modes work correctly
e2e_playwright/st_navigation.py Test app with dynamic navigation position switching and sidebar content

@sfc-gh-bnisco sfc-gh-bnisco merged commit 5eab4ee into develop Dec 5, 2025
66 of 67 checks passed
@sfc-gh-bnisco sfc-gh-bnisco deleted the 12-04-_fix_do_not_render_sidebar_nav_when_top_nav_enabled branch December 5, 2025 05:17
github-actions bot pushed a commit that referenced this pull request Dec 5, 2025
## Describe your changes

- Fixes a regression where both the Sidebar and Top nav would be visible
when navigation is set to `top`.

## GitHub Issue Link (if applicable)

Fixes #13224

## Testing Plan

- Adds e2e tests

---

**Contribution License Agreement**

By submitting this pull request you agree that all contributions to this
project are made under the Apache 2.0 license.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:bugfix PR contains bug fix implementation feature:st.navigation Related to `st.navigation` impact:users PR changes affect end users security-assessment-completed Security assessment has been completed for PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

With st.navigation(position="top"), the navigation appears in both the top and sidebar.

3 participants