fix(home): widget's "✕" Remove now actually removes the file
Reported: on the Home page after uploading data files, the Remove
buttons "on the right side" did nothing — the file kept showing up in
the list. That was the file_uploader widget's BUILT-IN ✕ icons (the
ones inside the uploader's chrome, on the right of each file row),
not our custom "Remove" buttons further down — the custom ones have
worked correctly since 84e4665.
Cause: ``_home_page`` deliberately treated the widget as add-only and
never honored widget-side removals. The reasoning, per the prior
comment, was that navigation can remount the widget with value ``[]``
— a render-time sync would then wipe ``home_uploads``. Real, but the
side effect was that the widget's own ✕ appeared to do nothing: the
file vanished from the widget chrome, stayed in ``home_uploads``, and
re-rendered immediately in the custom list below.
Fix: hook the file_uploader's ``on_change`` callback to reconcile
``home_uploads`` against the widget's current value. Streamlit's
``on_change`` fires ONLY on user-initiated value changes; the
remount-induced ``[]`` reset doesn't trigger it, so the stash still
survives navigation. Removals from the callback also drop the file's
findings entry and clear the singular ``home_uploaded_*`` keys when
the active upload was removed — matching the custom-button path.
The custom "Remove" buttons further down keep working unchanged; the
existing AppTest path through ``_home_remove_<sha1>`` still removes
exactly the file clicked. 2220 tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -36,6 +36,43 @@ class _StashedUpload:
|
||||
return self._data
|
||||
|
||||
|
||||
def _sync_uploader_to_home_uploads() -> None:
|
||||
"""``on_change`` callback for the home-page file_uploader.
|
||||
|
||||
Reconciles ``home_uploads`` (our persistent stash) with the widget's
|
||||
current value: adds newly-uploaded files, and drops files the user
|
||||
explicitly removed via the widget's built-in "✕" button. Per
|
||||
Streamlit semantics ``on_change`` only runs for user-initiated
|
||||
value changes, so the navigation-induced ``[]`` reset never reaches
|
||||
here — the stash survives intact across page switches.
|
||||
"""
|
||||
from src.audit import log_event
|
||||
|
||||
widget_files = st.session_state.get("home_upload") or []
|
||||
home_uploads: dict = st.session_state.setdefault("home_uploads", {})
|
||||
findings: dict = st.session_state.setdefault("home_findings_by_file", {})
|
||||
|
||||
widget_names = {f.name for f in widget_files}
|
||||
|
||||
for f in widget_files:
|
||||
if f.name not in home_uploads:
|
||||
home_uploads[f.name] = {"bytes": f.getvalue(), "size": f.size}
|
||||
log_event("upload", f"Uploaded {f.name}", filename=f.name, bytes=f.size)
|
||||
|
||||
for name in list(home_uploads.keys()):
|
||||
if name not in widget_names:
|
||||
del home_uploads[name]
|
||||
findings.pop(name, None)
|
||||
log_event("upload", f"Removed {name}", filename=name)
|
||||
if st.session_state.get("home_uploaded_name") == name:
|
||||
st.session_state.pop("home_uploaded_name", None)
|
||||
st.session_state.pop("home_uploaded_size", None)
|
||||
st.session_state.pop("home_uploaded_bytes", None)
|
||||
|
||||
st.session_state["home_uploads"] = home_uploads
|
||||
st.session_state["home_findings_by_file"] = findings
|
||||
|
||||
|
||||
def _home_page() -> None:
|
||||
"""Render the home page — multi-file upload + per-file analysis.
|
||||
|
||||
@@ -79,35 +116,26 @@ def _home_page() -> None:
|
||||
# Source of truth for uploaded files. dict[name -> {"bytes", "size"}].
|
||||
home_uploads: dict = st.session_state.setdefault("home_uploads", {})
|
||||
|
||||
# File uploader — for ADDING new files only. On every render we
|
||||
# merge widget-returned files INTO home_uploads but never remove
|
||||
# via the widget. (Widget state can return ``[]`` after navigation,
|
||||
# which we deliberately don't treat as "user cleared their files".)
|
||||
new_files = st.file_uploader(
|
||||
# File uploader — syncs into home_uploads via on_change. We deliberately
|
||||
# do NOT merge widget state into home_uploads at render time: navigation
|
||||
# can remount the widget with value ``[]``, and a render-time merge
|
||||
# would mistakenly leave home_uploads untouched while the user thinks
|
||||
# they're looking at empty state.
|
||||
#
|
||||
# ``on_change`` fires ONLY on user-initiated value changes (uploads
|
||||
# and the widget's built-in "✕" remove). It does NOT fire on the
|
||||
# remount-induced reset. That lets us treat the callback as ground
|
||||
# truth for both adds AND removes — fixing the previous bug where
|
||||
# the widget's "✕" appeared to do nothing because the file persisted
|
||||
# in home_uploads and immediately re-rendered in the list below.
|
||||
st.file_uploader(
|
||||
t("upload.uploader_label_multi"),
|
||||
type=["csv", "tsv", "xlsx", "xls"],
|
||||
accept_multiple_files=True,
|
||||
key="home_upload",
|
||||
help=t("upload.uploader_help"),
|
||||
on_change=_sync_uploader_to_home_uploads,
|
||||
)
|
||||
if new_files:
|
||||
from src.audit import log_event
|
||||
changed = False
|
||||
for f in new_files:
|
||||
if f.name not in home_uploads:
|
||||
home_uploads[f.name] = {
|
||||
"bytes": f.getvalue(),
|
||||
"size": f.size,
|
||||
}
|
||||
changed = True
|
||||
log_event(
|
||||
"upload",
|
||||
f"Uploaded {f.name}",
|
||||
filename=f.name,
|
||||
bytes=f.size,
|
||||
)
|
||||
if changed:
|
||||
st.session_state["home_uploads"] = home_uploads
|
||||
|
||||
# Persistent file list with per-file remove buttons. We render this
|
||||
# ourselves rather than trusting Streamlit's widget chrome because
|
||||
|
||||
Reference in New Issue
Block a user