From 888a8c380a1ec01b9780e95312099c9bc1d1d0da Mon Sep 17 00:00:00 2001 From: Jim Lancaster <40281233+zoltan57@users.noreply.github.com> Date: Tue, 11 Aug 2026 16:54:03 -0500 Subject: [PATCH] Continue GC code review: UI --- .github/instructions/ui.instructions.md | 6 +- docs/invariant/ui_style_guide.md | 16 +- src/transcription/ui/components/app_shell.py | 7 +- .../ui/components/data_display.py | 2 +- .../ui/components/document_panzoom.py | 48 +----- .../ui/components/error_presenter.py | 2 +- .../ui/components/table/common.py | 7 +- .../ui/components/table/documents.py | 11 +- src/transcription/ui/components/table/jobs.py | 20 +-- .../ui/components/table/sources.py | 13 +- src/transcription/ui/components/transcript.py | 19 +-- src/transcription/ui/components/viewers.py | 11 +- src/transcription/ui/static/theme.css | 161 ++++++++++++------ tests/test_ui_theme.py | 45 +++++ 14 files changed, 205 insertions(+), 163 deletions(-) diff --git a/.github/instructions/ui.instructions.md b/.github/instructions/ui.instructions.md index c72749d..9cb8528 100644 --- a/.github/instructions/ui.instructions.md +++ b/.github/instructions/ui.instructions.md @@ -40,10 +40,10 @@ Pages may depend on application services and framework-provided dependencies. Co ## CSS Assets -- Keep CSS under `ui/static` and split it into manageable, feature-oriented files. Do not grow a monolithic stylesheet or embed substantial style blocks in Python components. -- Load each stylesheet from the page, component, or composition root that needs it with `ui.add_css(...)`. Use shared registration only for genuinely application-wide styles. +- Keep all application CSS in `ui/static/theme.css`; do not add page- or component-specific stylesheets or embed style blocks in Python components. +- Load `theme.css` once from the composition root with `ui.add_css(..., shared=True)`. - Read stylesheet text through `importlib.resources.files(...)` so loading works from installed packages and is independent of the working directory. -- Centralize CSS reading in one typed helper cached by relative resource path with `functools.cache` or an equivalent unbounded `lru_cache`. Cache the immutable stylesheet text to avoid repeated resource I/O during component renders; keep NiceGUI registration decisions at the caller. +- Centralize CSS reading in one typed helper cached by resource path with `functools.cache` or an equivalent unbounded `lru_cache`. Cache the immutable stylesheet text to avoid repeated resource I/O; keep NiceGUI registration at the composition root. - Do not encode application behavior in CSS or other static assets. ## State and Side Effects diff --git a/docs/invariant/ui_style_guide.md b/docs/invariant/ui_style_guide.md index 84eeda7..54d63d6 100644 --- a/docs/invariant/ui_style_guide.md +++ b/docs/invariant/ui_style_guide.md @@ -65,12 +65,16 @@ Semantic tokens currently include: 4. ui-card-surface 5. ui-row-surface 6. ui-note-box +7. ui-card-error ### 5.3 Interactive Elements 1. ui-btn-primary 2. ui-btn-secondary 3. ui-link-primary 4. ui-text-accent +5. ui-chip-primary +6. ui-badge-secondary +7. ui-status and ui-status-- ### 5.4 Table Patterns 1. ui-table @@ -80,21 +84,15 @@ Semantic tokens currently include: Use existing class combinations from [src/transcription/ui/components](src/transcription/ui/components) and [src/transcription/ui/pages](src/transcription/ui/pages) as reference implementations. ## 6. Legacy Class Policy -Legacy classes with vibe- prefix still exist in a few components and are allowed only for compatibility while migrating: -1. Existing usage may remain temporarily. -2. New usage of vibe- classes is not allowed. -3. When touching a file that uses vibe- classes, prefer migrating it to ui- semantic classes in the same change when safe. - -Current legacy usage examples are in: -1. [src/transcription/ui/components/document_panzoom.py](src/transcription/ui/components/document_panzoom.py) -2. [src/transcription/ui/components/error_presenter.py](src/transcription/ui/components/error_presenter.py) -3. [src/transcription/ui/components/transcript.py](src/transcription/ui/components/transcript.py) +Legacy `vibe-` presentation classes are prohibited. Use `ui-` semantic classes from `theme.css`. ## 7. Prohibited Patterns 1. Inline hex colors in Python UI class strings or style blocks, except in isolated bridge code explicitly marked for migration. 2. Ad-hoc one-off class names that duplicate existing semantic class intent. 3. Page-specific palette forks that bypass theme tokens. 4. Hidden or low-contrast focus states on interactive controls. +5. Embedded ` - """, - shared=True, - ) def _document_url(source: Source) -> str: diff --git a/src/transcription/ui/components/error_presenter.py b/src/transcription/ui/components/error_presenter.py index 9772b85..4bb1d14 100644 --- a/src/transcription/ui/components/error_presenter.py +++ b/src/transcription/ui/components/error_presenter.py @@ -26,7 +26,7 @@ def show_error(exc: Exception, *, title: str, operation: str) -> None: close_button="Dismiss", ) - with ui.card().classes("vibe-card--error q-mt-md q-pa-md"): + with ui.card().classes("ui-card-error q-mt-md q-pa-md"): ui.label(title).classes("text-subtitle1") ui.label(error.message) ui.label(f"Suggested action: {error.suggestion}").classes("text-weight-medium") diff --git a/src/transcription/ui/components/table/common.py b/src/transcription/ui/components/table/common.py index bff5a7b..9b66923 100644 --- a/src/transcription/ui/components/table/common.py +++ b/src/transcription/ui/components/table/common.py @@ -4,7 +4,8 @@ import logging from collections.abc import Callable from typing import Any -from nicegui import events, ui +from nicegui import events +from nicegui import ui logger = logging.getLogger(__name__) @@ -61,7 +62,7 @@ def build_table( search_input = ( ui.input(placeholder=search_placeholder) .props("dense outlined clearable icon=search") - .classes("w-64 text-xs bg-white") + .classes("w-64 text-xs ui-form-surface") ) table = ( @@ -86,4 +87,4 @@ def build_table( if on_row_click_id is not None: _bind_row_click_handler(table, on_row_click_id=on_row_click_id) - return table \ No newline at end of file + return table diff --git a/src/transcription/ui/components/table/documents.py b/src/transcription/ui/components/table/documents.py index 84d3587..cab04e0 100644 --- a/src/transcription/ui/components/table/documents.py +++ b/src/transcription/ui/components/table/documents.py @@ -85,15 +85,14 @@ def render_documents_table(rows: Sequence[DocumentTableRow]) -> None: "body-cell-document_type", r""" - {{ props.value }} """, - ) \ No newline at end of file + ) diff --git a/src/transcription/ui/components/table/jobs.py b/src/transcription/ui/components/table/jobs.py index 65becf2..ff6bba2 100644 --- a/src/transcription/ui/components/table/jobs.py +++ b/src/transcription/ui/components/table/jobs.py @@ -4,7 +4,8 @@ from __future__ import annotations from collections.abc import Sequence from dataclasses import dataclass -from datetime import UTC, datetime +from datetime import UTC +from datetime import datetime from typing import Any from uuid import UUID @@ -12,6 +13,7 @@ from nicegui import ui from transcription.ui.components.cards import archival_card from transcription.ui.components.primitives import render_empty_state + from .common import build_table @@ -114,20 +116,14 @@ def render_jobs_table(rows: Sequence[JobTableRow]) -> None: "body-cell-status", r""" - {{ props.value.toUpperCase() }} """, - ) \ No newline at end of file + ) diff --git a/src/transcription/ui/components/table/sources.py b/src/transcription/ui/components/table/sources.py index 955935b..7488309 100644 --- a/src/transcription/ui/components/table/sources.py +++ b/src/transcription/ui/components/table/sources.py @@ -93,7 +93,7 @@ def render_sources_table(rows: Sequence[SourceTableRow]) -> None: "label": "Error Detail", "field": "job_source_error_detail", "sortable": False, - "classes": "font-mono text-xs truncate max-w-xs vibe-text-muted", + "classes": "font-mono text-xs truncate max-w-xs ui-text-muted", }, ], default_sort_by="page_number", @@ -105,15 +105,14 @@ def render_sources_table(rows: Sequence[SourceTableRow]) -> None: "body-cell-job_source_status", r""" - {{ props.value }} """, - ) \ No newline at end of file + ) diff --git a/src/transcription/ui/components/transcript.py b/src/transcription/ui/components/transcript.py index 4263be0..2b04e73 100644 --- a/src/transcription/ui/components/transcript.py +++ b/src/transcription/ui/components/transcript.py @@ -10,7 +10,6 @@ from typing import Any from nicegui import ui from transcription.db.models import Job -from transcription.db.models import JobSource from transcription.db.models import Source type RevisionAction = Callable[[Source], Awaitable[None] | None] @@ -25,21 +24,21 @@ def render_original_transcription_card(*, job: Job, classes: str = "w-full") -> model = job.model or "unknown" caption = f"{provider} | {model} | {_format_created_at(job.date_updated)}" - card = ui.card().classes(f"{classes} q-pa-md vibe-card") + card = ui.card().classes(f"{classes} q-pa-md ui-card-surface") with card, ui.column().classes("w-full q-gutter-y-sm"): ui.label(header).classes("text-subtitle1 text-weight-medium") - ui.label(caption).classes("text-caption vibe-text-muted") + ui.label(caption).classes("text-caption ui-text-muted") _metadata_row(label="Prompt", value=_latest_job_prompt(job) or "unknown") _metadata_row(label="Updated", value=_format_created_at(job.date_updated)) latest_transcription = _latest_job_transcription(job) if latest_transcription: - with ui.card().classes("w-full q-pa-sm"): + with ui.card().classes("w-full q-pa-sm ui-card-surface"): ui.markdown(latest_transcription) if latest_error_detail: - with ui.card().classes("w-full vibe-card--error q-pa-sm"): + with ui.card().classes("w-full ui-card-error q-pa-sm"): ui.label("Failure detail").classes("text-caption text-uppercase") ui.label(latest_error_detail).classes("text-body2") @@ -60,16 +59,16 @@ def render_revision_row( header = "Source revision | User-authored" caption = _format_created_at(revision.date_revised or revision.date_uploaded) - expansion = ui.expansion(value=initially_expanded, group="group").classes(f"{classes} rounded-borders vibe-card") + expansion = ui.expansion(value=initially_expanded, group="group").classes(f"{classes} ui-card-surface") with expansion, ui.column().classes("w-full q-gutter-y-sm q-pa-sm"): with expansion.add_slot("header"), ui.row().classes("w-full items-start justify-between q-gutter-md"): with ui.column().classes("q-gutter-none"): ui.label(header).classes("text-subtitle1 text-weight-medium") - ui.label(caption).classes("text-caption vibe-text-muted") + ui.label(caption).classes("text-caption ui-text-muted") if on_delete is not None: - with ui.dialog() as delete_dialog, ui.card().classes("q-pa-md"): + with ui.dialog() as delete_dialog, ui.card().classes("q-pa-md ui-card-surface"): ui.label("Delete this source revision?").classes("text-body1") with ui.row().classes("w-full justify-end q-gutter-sm"): ui.button("Cancel", on_click=lambda: delete_dialog.submit(False)).props("flat") @@ -94,7 +93,7 @@ def render_revision_row( _metadata_row(label="Created", value=_format_created_at(revision.date_revised or revision.date_uploaded)) if revision.revised_text: - with ui.card().classes("w-full q-pa-sm"): + with ui.card().classes("w-full q-pa-sm ui-card-surface"): ui.markdown(revision.revised_text) return expansion @@ -128,5 +127,5 @@ def _format_created_at(value: datetime) -> str: def _metadata_row(*, label: str, value: str) -> None: with ui.row().classes("w-md items-start justify-between q-gutter-x-md"): - ui.label(label).classes("text-caption vibe-text-muted text-uppercase") + ui.label(label).classes("text-caption ui-text-muted text-uppercase") ui.label(value).classes("text-body2 text-right break-all") diff --git a/src/transcription/ui/components/viewers.py b/src/transcription/ui/components/viewers.py index e375581..c4b4bc3 100644 --- a/src/transcription/ui/components/viewers.py +++ b/src/transcription/ui/components/viewers.py @@ -6,17 +6,16 @@ from nicegui import ui def dark_room_viewer( image_path: str | None, count_label: str = "1 Source Linked", - *, - container_height: str = "500px", ) -> None: """Render a plain responsive image that fills available width.""" del count_label if image_path: - ui.image(image_path).classes("w-full rounded-sm block").style("height: auto;") + ui.image(image_path).classes("w-full rounded-sm block ui-media-image") return with ui.column().classes( - "w-full items-center justify-center border ui-border-viewer ui-bg-viewer-overlay-soft rounded-sm p-8" - ).style(f"min-height: {container_height};"): - ui.label("No source media available for inspection.").classes("ui-text-muted text-xs italic") \ No newline at end of file + "w-full items-center justify-center border ui-border-viewer " + "ui-bg-viewer-overlay-soft rounded-sm p-8 ui-media-placeholder" + ): + ui.label("No source media available for inspection.").classes("ui-text-muted text-xs italic") diff --git a/src/transcription/ui/static/theme.css b/src/transcription/ui/static/theme.css index ae80223..a92a7c0 100644 --- a/src/transcription/ui/static/theme.css +++ b/src/transcription/ui/static/theme.css @@ -45,57 +45,6 @@ body { font-family: "Aptos", "Trebuchet MS", sans-serif; } -.q-card, -.vibe-card { - border: 1px solid var(--theme-border); - color: var(--theme-text); - background: var(--theme-surface-raised); - box-shadow: none; -} - -.vibe-card--error { - border-color: var(--palette-carbon-black); - color: var(--theme-inverse-text); - background: var(--palette-carbon-black); -} - -.vibe-text-muted { - color: var(--theme-text-muted); -} - -.vibe-separator { - background: var(--theme-border); -} - -.vibe-status { - border: 1px solid currentColor; -} - -.vibe-status--queued { - color: var(--palette-blue-slate); - background: var(--palette-platinum); -} - -.vibe-status--processing { - color: var(--palette-carbon-black); - background: var(--palette-powder-blue); -} - -.vibe-status--transcribed { - color: var(--palette-carbon-black); - background: var(--palette-cool-steel); -} - -.vibe-status--failed { - color: var(--palette-platinum); - background: var(--palette-carbon-black); -} - -.vibe-status--default { - color: var(--palette-blue-slate); - background: var(--theme-surface-muted); -} - button:focus-visible, a:focus-visible, textarea:focus-visible, @@ -105,7 +54,7 @@ input:focus-visible, outline-offset: 2px; } -/* Semantic utility classes for incremental migration away from inline hex styles. */ +/* Semantic utility classes */ .ui-text-primary { color: var(--theme-text); } @@ -163,6 +112,15 @@ input:focus-visible, color: var(--theme-text); background: var(--theme-surface-raised); border-radius: 0.125rem; + box-shadow: none; +} + +.ui-card-error { + border: 1px solid var(--theme-danger); + color: var(--theme-inverse-text); + background: var(--theme-danger); + border-radius: 0.125rem; + box-shadow: none; } .ui-row-surface { @@ -214,6 +172,45 @@ input:focus-visible, color: var(--theme-danger); } +.ui-chip-primary { + color: var(--theme-inverse-text); + background: var(--theme-primary); +} + +.ui-badge-secondary { + color: var(--theme-text); + background: var(--theme-secondary); +} + +.ui-status { + color: var(--theme-text-muted); + background: var(--theme-surface-muted); + border: 1px solid currentColor; +} + +.ui-status--queued, +.ui-status--pending { + color: var(--theme-primary); + background: var(--theme-surface-raised); +} + +.ui-status--processing { + color: var(--theme-text); + background: var(--theme-surface-muted); +} + +.ui-status--completed, +.ui-status--partial_success, +.ui-status--transcribed { + color: var(--theme-text); + background: var(--theme-secondary); +} + +.ui-status--failed { + color: var(--theme-inverse-text); + background: var(--theme-danger); +} + .ui-form-surface .q-field__control { background: var(--theme-surface-raised); } @@ -262,18 +259,18 @@ input:focus-visible, .ui-table .q-table th, .ui-table-header { - color: var(--theme-inverse-text) !important; - background-color: var(--theme-primary) !important; + color: var(--theme-inverse-text); + background-color: var(--theme-primary); font-weight: 700; } .ui-table .q-table td { - border-bottom: 1px solid var(--theme-border) !important; + border-bottom: 1px solid var(--theme-border); color: var(--theme-text); } .ui-table .q-table tbody tr:hover { - background-color: var(--theme-surface) !important; + background-color: var(--theme-surface); cursor: pointer; } @@ -304,10 +301,13 @@ input:focus-visible, } .app-shell__brand { + display: flex; + align-items: center; gap: 0.75rem; color: var(--theme-text); font-family: Georgia, serif; font-weight: 700; + text-decoration: none; } .app-shell__brand-mark { @@ -348,6 +348,55 @@ input:focus-visible, font-weight: 700; } +/* Media and source viewers */ +.ui-media-image { + height: auto; +} + +.ui-media-placeholder { + min-height: 31.25rem; +} + +.document-panzoom-filename { + max-width: 60%; + text-align: right; +} + +.document-panzoom-host { + height: min(70vh, 52rem); + overflow: hidden; + touch-action: none; + border: 1px solid var(--theme-viewer-border); + border-radius: 0.125rem; + background: var(--theme-viewer); +} + +.document-panzoom-surface { + width: 100%; + height: 100%; + display: flex; + align-items: flex-start; + justify-content: flex-start; +} + +.document-panzoom-media { + width: auto; + height: auto; + display: block; + max-width: 100%; + max-height: 100%; + user-select: none; + -webkit-user-drag: none; +} + +.document-panzoom-iframe { + width: 100%; + height: 100%; + border: 0; + pointer-events: none; + background: var(--theme-surface-raised); +} + @media (max-width: 700px) { .app-shell { padding-inline: 0.75rem; diff --git a/tests/test_ui_theme.py b/tests/test_ui_theme.py index e5bd702..fd369cf 100644 --- a/tests/test_ui_theme.py +++ b/tests/test_ui_theme.py @@ -1,6 +1,7 @@ """Tests for global UI theme registration.""" import re +from pathlib import Path import pytest from fastapi import FastAPI @@ -8,6 +9,8 @@ from fastapi import FastAPI from transcription.ui import register_pages from transcription.ui.resources import read_css +UI_ROOT = Path(__file__).parents[1] / "src" / "transcription" / "ui" + @pytest.mark.unit def test_page_registration_uses_vibescribe_theme(monkeypatch): @@ -35,3 +38,45 @@ def test_page_registration_uses_vibescribe_theme(monkeypatch): } assert "--q-primary" in theme_css assert run_options["dark"] is False + + +@pytest.mark.unit +def test_theme_is_the_only_ui_stylesheet(): + stylesheets = sorted(path.relative_to(UI_ROOT).as_posix() for path in UI_ROOT.rglob("*.css")) + + assert stylesheets == ["static/theme.css"] + + +@pytest.mark.unit +def test_ui_python_uses_class_driven_theme(): + prohibited_patterns = { + ".style(": "inline NiceGUI style", + "