-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Add NavigationContext
#12790
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
Add NavigationContext
#12790
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
✅ PR preview is ready!
|
NavigationContext & ScriptRunContext
NavigationContext & ScriptRunContextNavigationContext & SidebarConfigContext
NavigationContext & SidebarConfigContextNavigationContext
0f4bc7a to
62b6dc4
Compare
65881e8 to
49a5c47
Compare
62b6dc4 to
9ee3f05
Compare
49a5c47 to
84d2d68
Compare
9ee3f05 to
0a8b7fa
Compare
0a8b7fa to
e0e69c0
Compare
84d2d68 to
3d70b6e
Compare
e0e69c0 to
51c39dc
Compare
3d70b6e to
c9fb42e
Compare
📉 Frontend coverage change detectedThe frontend unit test (vitest) coverage has decreased by 0.0000%
✅ Coverage change is within normal range. |
There was a problem hiding this 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 introduces a new NavigationContext to separate multi-page app navigation concerns from LibContext and AppContext. The context consolidates navigation-related properties (currentPageScriptHash, onPageChange, pageLinkBaseUrl, navSections, appPages) into a dedicated context provider, improving separation of concerns and making the component structure cleaner.
Key changes:
- Creates
NavigationContextwith navigation-specific properties previously spread across contexts - Updates
StreamlitContextProviderto include the newNavigationContextprovider - Migrates components (
PageLink,Sidebar,SidebarNav,TopNav,AppView) to consume fromNavigationContext
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
frontend/lib/src/components/core/NavigationContext.tsx |
New context definition with navigation properties and comprehensive documentation |
frontend/lib/src/components/core/LibContext.tsx |
Removed onPageChange and currentPageScriptHash properties now in NavigationContext |
frontend/app/src/components/AppContext.tsx |
Removed navigation-related properties (pageLinkBaseUrl, appPages, etc.) |
frontend/app/src/components/StreamlitContextProvider.tsx |
Added NavigationContext provider to context hierarchy |
frontend/lib/src/index.ts |
Exported NavigationContext and its types |
frontend/lib/src/test_util.tsx |
Updated test utilities to support NavigationContext in provider hierarchy |
frontend/lib/src/components/elements/PageLink/* |
Updated to consume navigation state from NavigationContext |
frontend/app/src/components/Navigation/* |
Updated navigation components to use NavigationContext |
frontend/app/src/components/Sidebar/* |
Updated sidebar components to consume from NavigationContext |
frontend/app/src/components/AppView/* |
Removed navigation props in favor of context consumption |
| Various test files | Updated tests to accommodate new context parameter order |
📈 Python coverage change detectedThe Python unit test coverage has increased by 0.0100%
✅ Coverage change is within normal range. Coverage by files
|
51c39dc to
82c26a7
Compare
f15c4cb to
2bd1ffb
Compare
2bd1ffb to
ac1148f
Compare
82c26a7 to
4b7aa49
Compare
ac1148f to
4ba6af1
Compare
4b7aa49 to
18e4daf
Compare
18e4daf to
d7551c7
Compare
4ba6af1 to
352b359
Compare
352b359 to
1985977
Compare

Describe your changes
Part 3 of
StreamlitContextProviderupdatesAdds new
NavigationContextwith the following:currentPageScriptHashonPageChangepageLinkBaseUrlnavSectionsappPagesTesting Plan