Skip to content

feat(bookmark): Support bookmarking input files #1945

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 11 commits into from
Mar 28, 2025
Merged

Conversation

schloerke
Copy link
Collaborator

@schloerke schloerke commented Mar 28, 2025

@schloerke schloerke marked this pull request as ready for review March 28, 2025 20:26
@schloerke schloerke requested a review from Copilot March 28, 2025 20:26
Copy link
Contributor

@Copilot 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 introduces support for bookmarking file input values by switching the bookmark store from "url" to "server" and adding logic to restore file inputs. Key changes include:

  • Updating the configuration in the Express app to use server-based bookmarking.
  • Enhancing file input restoration and serialization in the UI, session, input handler, and serializer modules.
  • Adjusting test behavior by removing the skip marker and updating project configuration for temporary file handling.

Reviewed Changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/playwright/ai_generated_apps/bookmark/input_file/test_input_file_express_bookmarking.py Removed skip marker to enable test execution
tests/playwright/ai_generated_apps/bookmark/input_file/app-express.py Changed bookmark_store from "url" to "server"
shiny/ui/_input_file.py Added restored value handling and associated warning for file input restoration
shiny/session/_session.py Configured serializer assignment during file input upload
shiny/input_handler.py Added error handling for non-server bookmark_store and temporary directory usage for file restoration
shiny/bookmark/_serializers.py Updated the file input serializer logic to work with a list of file info dicts
shiny/bookmark/_bookmark_state.py Centralized the bookmark folder name for consistency
shiny/_main.py Updated reload exclusions to ignore the bookmark folder and additional virtual environment folders
pyproject.toml Added a custom uvicorn source branch to support reload exclusion of absolute paths
Files not reviewed (1)
  • MANIFEST.in: Language not supported
Comments suppressed due to low confidence (2)

tests/playwright/ai_generated_apps/bookmark/input_file/test_input_file_express_bookmarking.py:9

  • [nitpick] The removal of the skip marker means this test will now run; ensure that the test passes reliably or conditionally skip it if issues persist.
-@pytest.mark.skip("Broken test! TODO: Barret")

shiny/input_handler.py:246

  • [nitpick] Creating a new temporary directory for each file input may introduce inefficiencies; consider reusing a single temporary directory to handle multiple files.
tempdir_root = tempfile.TemporaryDirectory()

@schloerke schloerke enabled auto-merge (squash) March 28, 2025 20:40
@schloerke schloerke merged commit ca671cd into main Mar 28, 2025
54 checks passed
@schloerke schloerke deleted the bookmark_file_input branch March 28, 2025 20:41
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant