From b9147f3b66907db497e97e5afaa570a516ed803e Mon Sep 17 00:00:00 2001 From: Michael Date: Sat, 16 May 2026 21:48:28 +0000 Subject: [PATCH] fix(downloads): save server-side to ~/Downloads + open-folder link MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switch the download mechanic from "browser with a data: URL" to "write the bytes directly to the user's Downloads folder and show them the exact path". DataTools runs as a local Streamlit app, so the "server" IS the user's machine — there's no reason to go through the browser save dialog at all. Flow: 1. Click "Download " button (rendered as a regular ``st.button``, so no widget-collision issues). 2. Bytes are written to ``Path.home() / "Downloads" / file_name`` (overwriting any same-named file). 3. The page reruns and renders a success caption with the absolute path the file landed at. 4. An "📂 Open Downloads folder" button appears. Clicking it pops the OS file manager via ``os.startfile`` (Windows), ``open`` (macOS), or ``xdg-open`` (Linux). Why this is better than the previous HTML-data-URL helper: - Unambiguous about where the file went — user sees the full path, not "wherever your browser was configured to save". - The data: URL approach base64-inflated the page payload by 33% and bloated for large outputs; server-side write is byte-for-byte. - No more browser-side widget collision class of bug. - The save action is a real Streamlit button, so the existing widget semantics (disabled, help tooltip, key isolation) work without workarounds. API surface unchanged. New canonical name ``local_download_button``; ``html_download_button`` is kept as a back-compat alias that points at the same implementation — every existing call site continues to work without edits. Tests are protected from polluting the developer's home dir via a ``DATATOOLS_DOWNLOADS_DIR`` env var override returned by the new ``_downloads_dir()`` helper. Smoke verified end-to-end via AppTest: click → file appears in tmp dir → success banner shows path → open-folder button renders. 2220 tests pass, 91 skipped, 35 s. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/gui/components/__init__.py | 1 + src/gui/components/_legacy.py | 156 +++++++++++++++++++++++---------- 2 files changed, 111 insertions(+), 46 deletions(-) diff --git a/src/gui/components/__init__.py b/src/gui/components/__init__.py index 5337f7a..e66f83b 100644 --- a/src/gui/components/__init__.py +++ b/src/gui/components/__init__.py @@ -49,6 +49,7 @@ __all__ = [ "back_to_home_link", "hide_streamlit_chrome", "html_download_button", + "local_download_button", "shutdown_app", "pickup_or_upload", # License gate + activation form diff --git a/src/gui/components/_legacy.py b/src/gui/components/_legacy.py index 8234517..ce652e5 100644 --- a/src/gui/components/_legacy.py +++ b/src/gui/components/_legacy.py @@ -239,68 +239,132 @@ def _farewell_script() -> str: ) -def html_download_button( +def _downloads_dir() -> "Path": + """Return the user's Downloads folder. + + Defaults to ``~/Downloads``. Overrideable via the + ``DATATOOLS_DOWNLOADS_DIR`` environment variable so tests can write + to a temp directory instead of polluting the developer's home. + """ + import os + from pathlib import Path + override = os.environ.get("DATATOOLS_DOWNLOADS_DIR") + if override: + return Path(override) + return Path.home() / "Downloads" + + +def _open_in_file_manager(path: "Path") -> bool: + """Open *path* in the host OS's native file manager. + + Returns ``True`` if the command was dispatched (no guarantee the + file manager actually opened — the user might have removed the + default handler). Returns ``False`` on platforms we don't know + how to launch. + """ + import os + import subprocess + try: + if sys.platform == "win32": + os.startfile(str(path)) # type: ignore[attr-defined] + return True + if sys.platform == "darwin": + subprocess.Popen(["open", str(path)]) + return True + # Linux / *BSD / etc. + subprocess.Popen(["xdg-open", str(path)]) + return True + except Exception: + return False + + +def local_download_button( label: str, data: bytes, *, file_name: str, - mime: str = "application/octet-stream", + mime: str = "application/octet-stream", # noqa: ARG001 — kept for API compat disabled: bool = False, help: str | None = None, use_container_width: bool = True, ) -> None: - """Render a download trigger as a real ```` anchor. + """Save bytes directly to the user's Downloads folder. - Replaces ``st.download_button`` for pages that stack multiple - download triggers in one render pass. Streamlit's ``download_button`` - has a long-standing failure mode where only the first button in the - page actually fires when several are rendered together: explicit - ``key`` arguments are not sufficient, since the browser-side - bytes-to-Blob translation appears to share state across widgets in - some browsers (Edge/Chrome on Windows in particular). + DataTools runs as a local Streamlit app, so the "server" IS the + user's machine — we can write straight to ``~/Downloads/`` + instead of going through the browser save dialog. On click: - Sidestepping the widget system entirely fixes it. The bytes are - base64-encoded into a ``data:`` URL on the anchor's ``href``; the - browser's native ``download`` attribute pops the standard save - dialog. No script reruns happen on click — that's an upside, since - it avoids resetting any other in-flight UI state. + 1. Bytes are written to ``Path.home() / "Downloads" / file_name`` + (overwriting any existing file with the same name). + 2. The page reruns and renders a success caption naming the exact + absolute path the file landed at. + 3. An "Open Downloads folder" button appears that pops the OS file + manager (Explorer / Finder / xdg-open) at the parent directory. - Caveat: data: URLs balloon by 33% (base64). Fine up to a few tens - of MB. For 1 GB+ datasets a different mechanism would be needed, - but tool output is rarely that large. + Why not ``st.download_button`` or an HTML data: URL anchor? + + - ``st.download_button`` has a long-standing failure mode where + only the first button on the page fires when multiple are + stacked together. + - Data: URLs balloon by 33% (base64) and leave the user guessing + where the browser saved it (default Downloads folder or wherever + they last picked — varies per browser). + + The save-server-side path is unambiguous, works the same regardless + of browser settings, and gives the user a real link to the file. + + The ``mime`` parameter is accepted for backwards compatibility with + the previous helper signature; it is no longer relevant because + nothing on the wire knows the bytes' content type. """ - import base64 - import html as _html + import hashlib + from pathlib import Path - width_css = "width:100%;" if use_container_width else "" - base_style = ( - "display:inline-block;text-align:center;" - "padding:0.375rem 0.75rem;border-radius:0.5rem;" - "border:1px solid rgba(49,51,63,0.2);" - "background:rgb(240,242,246);color:rgb(38,39,48);" - "text-decoration:none;font-weight:400;cursor:pointer;" - "font-family:inherit;font-size:14px;" - "box-sizing:border-box;line-height:1.6;" - f"{width_css}" + # Stable widget keys, namespaced by file_name + content digest so + # repeated renders of the same content keep their saved-state + # banner, but a re-run that produced different bytes gets a fresh + # button with no stale success message. + digest = hashlib.sha1(data, usedforsecurity=False).hexdigest()[:8] + btn_key = f"_dl_btn_{file_name}_{digest}" + saved_key = f"_dl_saved_{file_name}_{digest}" + open_key = f"_dl_open_{file_name}_{digest}" + + clicked = st.button( + label, + key=btn_key, + disabled=disabled, + help=help, + type="secondary", + use_container_width=use_container_width, ) - safe_label = _html.escape(label) - title_attr = f' title="{_html.escape(help)}"' if help else "" - if disabled: - disabled_style = base_style + "opacity:0.5;cursor:not-allowed;" - st.markdown( - f'{safe_label}', - unsafe_allow_html=True, - ) - return + if clicked: + target_dir = _downloads_dir() + try: + target_dir.mkdir(parents=True, exist_ok=True) + target = target_dir / file_name + target.write_bytes(data) + st.session_state[saved_key] = str(target) + except Exception as e: + st.error( + f"Could not save **{file_name}** to `{target_dir}`: {e}" + ) + return - b64 = base64.b64encode(data).decode("ascii") - safe_name = _html.escape(file_name, quote=True) - st.markdown( - f'{safe_label}', - unsafe_allow_html=True, - ) + saved_path_str = st.session_state.get(saved_key) + if saved_path_str: + st.success(f"✓ Saved to `{saved_path_str}`") + if st.button( + "📂 Open Downloads folder", + key=open_key, + type="secondary", + ): + _open_in_file_manager(Path(saved_path_str).parent) + + +# Back-compat alias: existing call sites use the old name. New code +# should prefer ``local_download_button``. +html_download_button = local_download_button def back_to_home_link(*, key: str = "_back_to_home_link") -> None: