-
Notifications
You must be signed in to change notification settings - Fork 4k
Support passing query params to st.page_link
#13093
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
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. |
✅ PR preview is ready!
|
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 adds support for passing query parameters to st.page_link, enabling developers to navigate to pages with pre-set query parameters for both internal multipage apps and external URLs.
Key Changes
- Added
query_stringfield to thePageLinkprotobuf message - Implemented
process_query_params()helper function to convert query param dicts/iterables into URL-encoded strings - Updated frontend to append query params to external link hrefs and pass them through
onPageChangefor internal navigation
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| proto/streamlit/proto/PageLink.proto | Added query_string field to PageLink message for storing URL-encoded query parameters |
| lib/streamlit/runtime/state/query_params.py | Added process_query_params() and _set_item_in_dict() helper functions to process and validate query params |
| lib/streamlit/elements/widgets/button.py | Added query_params parameter to page_link() and _page_link() methods with processing logic |
| lib/tests/streamlit/runtime/state/query_params_test.py | Added comprehensive unit tests for process_query_params() covering dicts, iterables, list values, type conversions, and validation |
| lib/tests/streamlit/elements/page_link_test.py | Added test for query_params parameter with external URL |
| frontend/lib/src/components/elements/PageLink/PageLink.tsx | Updated to append query string to external link hrefs and pass to onPageChange for internal navigation |
| frontend/lib/src/components/core/NavigationContext.tsx | Updated onPageChange signature to accept optional queryString parameter |
| frontend/app/src/App.tsx | Updated onPageChange and sendRerunBackMsg to handle query string overrides with proper embed param preservation |
| e2e_playwright/multipage_apps_v2/mpa_v2_basics.py | Added test page link with query params |
| e2e_playwright/multipage_apps_v2/mpa_v2_basics_test.py | Added E2E test verifying query params are properly set when navigating via page link |
st.page_linkst.page_link
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
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
st.page_linkst.page_link
frontend/app/src/App.tsx
Outdated
| queryString = preservedQueryParams | ||
| this.setState({ queryParams: preservedQueryParams }) | ||
|
|
||
| if (queryStringOverride !== undefined) { |
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.
[nit] This could be a bit easier to read if it were separated out into a function so that the reader can get a high level idea of what it happening based on the function name.
E.g.
queryString = getQueryString(queryStringOverride, preservedQueryParams)
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.
Moved into helper function 👍
| } | ||
| } | ||
|
|
||
| let href = element.page |
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.
[nit] It would also be more readable here to separate this into a function.
const href = buildHref(element)
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.
Moved into helper function 👍
| """Set an item in a dictionary.""" | ||
| if isinstance(value, dict): | ||
| raise StreamlitAPIException( | ||
| f"You cannot set a query params key `{key}` to a dictionary." |
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.
[nit] This isn't new, but I find this error message phrasing odd. Should it be more like, "Query params cannot be a dictionary. Provide a string or iterable as the value for {key}"
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.
Update the error messgae 👍
| if key in processed_params: | ||
| # If the key already exists, we need to accumulate the values. | ||
| if isinstance(value, dict): | ||
| raise StreamlitAPIException( |
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.
[suggestion] Maybe this should be defined as a specific error message since it is used in two places to keep the message consistent.
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.
Updated the error and reused the message 👍
sfc-gh-lwilby
left a comment
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.
LGTM, left some minor comments.
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
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Describe your changes
Adds support for passing query parameters to
st.page_link:GitHub Issue Link (if applicable)
Testing Plan
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.