From c568aec8a74f53c938883c626f86fddb9b7d30c2 Mon Sep 17 00:00:00 2001 From: Michael Date: Sat, 16 May 2026 20:17:14 +0000 Subject: [PATCH] feat(gui): one-click Close in its own bottom sidebar section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Close is now a direct shutdown trigger: visiting the Close page (the sidebar entry) fires shutdown_app() immediately — no confirm step, no intermediate body. The farewell overlay paints and os._exit(0) lands ~1s later from a daemon thread. Layout: Close moved into its own bottom-of-sidebar section so the destructive action is visually separated from Account/Activate. - New shutdown_app() in components/_legacy.py replaces quit_button. os._exit thread is skipped when "pytest" is in sys.modules so the test suite doesn't suicide on rendering 99_Close. - pages/99_Close.py shrinks to set_page_config + chrome + shutdown_app. - app.py nav grows a new "Close" section header (new nav.section_close key in en/es packs) pinned at the bottom of the navigation dict. Tests updated: - TestQuitButtonRenders → TestClosePageShutsDownImmediately. Assert the shutdown caption renders + no confirm button exists. - test_smoke EXPECTED_SUBSTRINGS["99_Close"] now pins "Shutting down" / "Cerrando" (the visible page body) instead of the removed page title. 2008 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/gui/app.py | 7 +++++- src/gui/components/__init__.py | 2 +- src/gui/components/_legacy.py | 46 ++++++++++++++++++---------------- src/gui/pages/99_Close.py | 22 ++++++---------- src/i18n/packs/en.json | 3 ++- src/i18n/packs/es.json | 3 ++- tests/gui/test_chrome.py | 37 ++++++++++++--------------- tests/gui/test_smoke.py | 2 +- 8 files changed, 59 insertions(+), 63 deletions(-) diff --git a/src/gui/app.py b/src/gui/app.py index 1170dcc..bc6b00d 100644 --- a/src/gui/app.py +++ b/src/gui/app.py @@ -198,12 +198,17 @@ def _build_navigation() -> dict[str, list]: ) account_header = _t("nav.section_account") or "Account" + close_header = _t("nav.section_close") or "Close" + # Close lives in its own section pinned at the very bottom of the + # sidebar — its own click is the shutdown, so we visually separate + # it from the navigable pages above to reduce mis-click risk. return { "": [home], section_label("cleaners"): by_section["cleaners"], section_label("transformations"): by_section["transformations"], section_label("automations"): by_section["automations"], - account_header: [activate, close], + account_header: [activate], + close_header: [close], } diff --git a/src/gui/components/__init__.py b/src/gui/components/__init__.py index 52edd24..cc64258 100644 --- a/src/gui/components/__init__.py +++ b/src/gui/components/__init__.py @@ -47,7 +47,7 @@ from .activation import ( # noqa: F401 re-exported __all__ = [ # Shared chrome / pickup "hide_streamlit_chrome", - "quit_button", + "shutdown_app", "pickup_or_upload", # License gate + activation form "render_activation_form", diff --git a/src/gui/components/_legacy.py b/src/gui/components/_legacy.py index 14740f5..7f75775 100644 --- a/src/gui/components/_legacy.py +++ b/src/gui/components/_legacy.py @@ -4,6 +4,7 @@ from __future__ import annotations import io import os +import sys import threading import time from typing import Optional @@ -186,37 +187,38 @@ def _farewell_script() -> str: ) -def quit_button(label: str | None = None, *, key: str = "quit_app_button") -> None: - """Render a Quit button that terminates the Streamlit server. +def shutdown_app() -> None: + """Terminate the Streamlit server immediately, no confirm. + + Designed to be called from a page whose mere act of rendering means + the user wants to quit (e.g., the sidebar Close entry). Schedules + ``os._exit(0)`` on a daemon thread so the process terminates after + the farewell overlay has had a chance to paint, then injects the + overlay JS and short-circuits the rest of the page via ``st.stop``. Streamlit has no first-class shutdown hook, and signalling the process (SIGTERM/SIGINT) does not reliably terminate it — Streamlit installs its own handlers and the tornado/asyncio loop swallows or defers the signal, so the browser sees the websocket drop while the - python process stays alive. We schedule ``os._exit(0)`` on a daemon - thread to hard-kill the process, and inject a small JS shim that - either closes the tab (when allowed) or replaces the top frame with - a self-contained "App closed" page so the user never sees - Streamlit's red connection-error overlay. + python process stays alive. ``os._exit`` is the only reliable kill. + + The hard-exit thread is skipped under pytest so the test suite does + not suicide when a test renders this page. The overlay + caption + still render so test assertions about content work. """ - if label is None: - label = _t("quit.button") - - if st.session_state.get("_app_shutting_down"): - from streamlit.components.v1 import html as _components_html - _components_html(_farewell_script(), height=0) - st.success(_t("quit.shutting_down")) - st.stop() - - if st.button(label, key=key, type="secondary"): + if not st.session_state.get("_app_shutting_down"): st.session_state["_app_shutting_down"] = True + if "pytest" not in sys.modules: + def _hard_exit() -> None: + time.sleep(1.0) + os._exit(0) - def _hard_exit() -> None: - time.sleep(1.0) - os._exit(0) + threading.Thread(target=_hard_exit, daemon=True).start() - threading.Thread(target=_hard_exit, daemon=True).start() - st.rerun() + from streamlit.components.v1 import html as _components_html + _components_html(_farewell_script(), height=0) + st.success(_t("quit.shutting_down")) + st.stop() # --------------------------------------------------------------------------- diff --git a/src/gui/pages/99_Close.py b/src/gui/pages/99_Close.py index f8d3af8..9479ba3 100644 --- a/src/gui/pages/99_Close.py +++ b/src/gui/pages/99_Close.py @@ -1,9 +1,9 @@ -"""Close — shut the DataTools server down cleanly. +"""Close — shut the DataTools server down immediately on visit. -Lives in the sidebar nav alongside the tool pages so users have a -discoverable way to terminate the local Streamlit process without -having to Ctrl+C in the shell. An explicit confirm step prevents an -accidental sidebar click from killing a session mid-work. +Sidebar nav entry. Clicking it routes here, and the page's render +fires :func:`shutdown_app` directly — there is no confirm step. The +browser sees the farewell overlay; the Python process terminates a +moment later via ``os._exit``. """ from __future__ import annotations @@ -17,7 +17,7 @@ _project_root = Path(__file__).resolve().parent.parent.parent.parent if str(_project_root) not in sys.path: sys.path.insert(0, str(_project_root)) -from src.gui.components import hide_streamlit_chrome, quit_button +from src.gui.components import hide_streamlit_chrome, shutdown_app from src.i18n import t st.set_page_config( @@ -25,14 +25,6 @@ st.set_page_config( page_icon="🛑", layout="wide", ) - hide_streamlit_chrome() -st.title(t("close_page.title")) -st.caption(t("close_page.caption")) -st.divider() - -st.markdown(t("close_page.body")) - -st.write("") -quit_button(label=t("close_page.button"), key="quit_app_button_page") +shutdown_app() diff --git a/src/i18n/packs/en.json b/src/i18n/packs/en.json index 8cabe77..81f5d25 100644 --- a/src/i18n/packs/en.json +++ b/src/i18n/packs/en.json @@ -148,6 +148,7 @@ "home_page_title": "Home", "section_account": "Account", "activate_title": "Activate", - "close_title": "Close" + "close_title": "Close", + "section_close": "Close" } } diff --git a/src/i18n/packs/es.json b/src/i18n/packs/es.json index 95cdaec..a62f4c6 100644 --- a/src/i18n/packs/es.json +++ b/src/i18n/packs/es.json @@ -148,6 +148,7 @@ "home_page_title": "Inicio", "section_account": "Cuenta", "activate_title": "Activar", - "close_title": "Cerrar" + "close_title": "Cerrar", + "section_close": "Cerrar" } } diff --git a/tests/gui/test_chrome.py b/tests/gui/test_chrome.py index dc9e516..73d662f 100644 --- a/tests/gui/test_chrome.py +++ b/tests/gui/test_chrome.py @@ -122,38 +122,33 @@ class TestLocalizedChrome: # Quit / Close page # --------------------------------------------------------------------------- -class TestQuitButtonRenders: - """The Close page must show the localized title, body, and the - Close-the-app button. We don't actually click the button — that - would call ``os._exit(0)`` and kill the test process. We only - assert the button is present and its label is localized.""" +class TestClosePageShutsDownImmediately: + """The Close page no longer renders a confirm button — visiting the + page IS the close action. Under pytest the ``os._exit`` thread is + skipped (so the test runner doesn't suicide), but the farewell + success caption still renders and we assert against it.""" - def test_close_page_english(self, app_factory): + def test_english_close_renders_shutdown_message(self, app_factory): app = app_factory("99_Close") app.run() text = collected_text(app) - assert "Close DataTools" in text + assert "Shutting down" in text or "Goodbye" in text, ( + f"Close page missing shutdown caption; got:\n{text[:300]}" + ) + # No confirm button — the page is the action. labels = [b.label for b in app.button] - assert any("Close the app" in lbl for lbl in labels), ( - f"Close-the-app button missing; buttons: {labels}" + assert not any("Close the app" in lbl for lbl in labels), ( + f"Close page should not render a confirm button; got: {labels}" ) - def test_close_page_spanish(self, app_factory): + def test_spanish_close_renders_localized_shutdown_message(self, app_factory): app = app_factory("99_Close") with_language(app, "es") app.run() text = collected_text(app) - assert "Cerrar DataTools" in text - labels = [b.label for b in app.button] - assert any("Cerrar la app" in lbl for lbl in labels), ( - f"Spanish Close button missing; buttons: {labels}" + # ``quit.shutting_down`` in the es pack. + assert "Cerrando" in text or "Apagando" in text or "Adiós" in text, ( + f"Spanish Close page missing localized shutdown caption; got:\n{text[:300]}" ) - def test_close_body_describes_unsaved_work_warning_es(self, app_factory): - app = app_factory("99_Close") - with_language(app, "es") - app.run() - text = collected_text(app) - assert "trabajo sin guardar" in text - diff --git a/tests/gui/test_smoke.py b/tests/gui/test_smoke.py index 94038e9..e2da6b2 100644 --- a/tests/gui/test_smoke.py +++ b/tests/gui/test_smoke.py @@ -61,7 +61,7 @@ EXPECTED_SUBSTRINGS: dict[str, dict[str, str]] = { "7_Multi_File_Merger": {"en": "Combine Files", "es": "Combine Files"}, "8_Validator_Reporter": {"en": "Quality Check", "es": "Quality Check"}, "9_Pipeline_Runner": {"en": "Automated", "es": "Automated"}, - "99_Close": {"en": "Close DataTools", "es": "Cerrar DataTools"}, + "99_Close": {"en": "Shutting down", "es": "Cerrando"}, }