Skip to content

UI consistency pass: shared header/section/dialog helpers across views#244

Open
NKeleher wants to merge 5 commits into
mainfrom
ux-review
Open

UI consistency pass: shared header/section/dialog helpers across views#244
NKeleher wants to merge 5 commits into
mainfrom
ux-review

Conversation

@NKeleher

@NKeleher NKeleher commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Pull Request Summary 🚀

What does this PR do? 📝

Improves UI consistency and reduces clutter across the DataSure Streamlit views, without changing the data pipeline or check logic. It introduces a shared UI-helpers module and routes every view's headers, dividers, and destructive-action confirmations through it.

Highlights:

  • New utils/ui_utils.py with four helpers: page_header, section_header, metric_row, and a single confirm_dialog (st.dialog-based) idiom.
  • Nav ↔ title alignment: in-page headings now match their sidebar nav labels (e.g. "Get Your Data Ready" → Prepare Data, "Replication Package" → Export Replication Package), each with a short subtitle.
  • Unified confirmations: demo restart, project delete, remove-import-config, remove-prep-step, and remove-correction now all use the same modal dialog (replacing four different ad hoc idioms).
  • Standardized dividers: every st.write("---")st.divider().
  • Clutter removed: dead disabled credential selectbox (import), double-nested border (correction), three stacked PII callouts collapsed to one + :white_check_mark::material/check_circle: (replication), empty _render_demo_guidance stub (config), and a bordered import-config card matching the credentials card.
  • Bug fix: added a key to the "Select Data to Remove" selectbox so its choice persists across reruns (the Remove button was previously never enabled), plus a disabled guard preventing a delete with no selection.

Why is this change needed? 🤔

The seven view files were built independently over time, so headings, dividers, confirmation patterns, and layout drifted apart. My audit with Claude found ~11 cross-view inconsistencies plus several clutter spots. This made the app read as several loosely related pages rather than one coherent product. Centralizing the repeated patterns fixes that and makes future views consistent by default.

How was this implemented? 🛠️

  • Added src/datasure/utils/ui_utils.py. The helpers import streamlit at call time so they honor the view-test harness's sys.modules swap regardless of import order.
  • Refactored the view scripts (start_view, import_view, prep_view, config_view, correction_view, replication_view, output_view_template) to call the helpers instead of ad hoc st.title/st.subheader/st.write("---")/bespoke confirmations.
  • No changes to checks/, connectors, processing, or .streamlit/config.toml (the IPA theme was already solid).
  • st.form conversion and the nine-tab output restructure were deliberately left out of scope (documented as a future follow-up).

How to test or reproduce ? 🧪

  1. uv run python -m pytest -m "not slow" — full suite (3093 passed, 7 pre-existing skips).
  2. uv tool run pre-commit run --all-files — all hooks pass.
  3. uv run datasure and walk the flow:
    • Each page's heading matches its sidebar nav label, with a subtitle and a single divider style.
    • Start a demo → Restart Demo and project delete open a modal confirm with Confirm/Cancel.
    • Import page: no disabled dead credential option; credentials and import-config both render as bordered cards.
    • Replication page: a single PII notice gating a checkbox-guarded download; progress uses Material check icons.

Screenshots (if applicable) 📷

Confirmation that demo should be restarted:
image

Require confirmation if configuration to be deleted:
image

Checklist ✅

  • I have run and tested my changes locally
  • I have limit this PR to less than 1000 lines of code change (if not, explain why)
  • I have updated/added tests to cover my changes (if applicable)
  • I have updated/added requirements to cover my changes (if applicable)
  • I have run linting and formatting on any code changes (if applicable)
  • I have updated the documentation (README, etc.) accordingly
  • I have reviewed and resolved any merge conflict

@NKeleher
NKeleher requested a review from a team as a code owner July 17, 2026 05:29
@sonarqubecloud

Copy link
Copy Markdown

@NKeleher NKeleher mentioned this pull request Jul 17, 2026
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant