Continue GC code review: UI

This commit is contained in:
Jim Lancaster
2026-08-11 16:54:03 -05:00
parent b8be27f0c9
commit 888a8c380a
14 changed files with 205 additions and 163 deletions
+3 -3
View File
@@ -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
+7 -9
View File
@@ -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--<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 `<style>` blocks or NiceGUI `.style(...)` calls in Python UI code.
6. Additional page- or component-specific stylesheets; `theme.css` is the single CSS source.
## 8. Implementation Rules For Contributors
1. Prefer composing existing semantic classes before creating new ones.
+3 -4
View File
@@ -4,6 +4,8 @@ from __future__ import annotations
from nicegui import ui
from transcription.ui.theme import VIBESCRIBE_LOGO_SVG
NAV_ITEMS: tuple[tuple[str, str, str], ...] = (
("Documents", "/documents", "description"),
("People", "/people", "group"),
@@ -11,7 +13,6 @@ NAV_ITEMS: tuple[tuple[str, str, str], ...] = (
("Jobs", "/jobs", "work_history"),
)
from transcription.ui.theme import VIBESCRIBE_LOGO_SVG
def _is_active_path(*, current_path: str, item_path: str) -> bool:
if item_path == "/jobs":
@@ -50,9 +51,7 @@ def render_app_shell(*, current_path: str | None = None) -> None:
normalized_path = _normalize_path(current_path)
with ui.header().classes("app-shell"), ui.element("div").classes("app-shell__inner"):
with ui.element("a").props('href="/ui/homepage"').style(
"display:flex; align-items:center; gap:0.75rem; text-decoration:none; color:inherit;"
).classes("app-shell__brand no-wrap"):
with ui.element("a").props('href="/ui/homepage"').classes("app-shell__brand no-wrap"):
ui.html(VIBESCRIBE_LOGO_SVG).classes("app-shell__brand-mark")
ui.label("VibeScribe").classes("app-shell__brand-name")
@@ -11,4 +11,4 @@ def metadata_row(label: str, value: str):
def archival_badge(text: str):
"""Standardized Aged Sepia badge."""
return ui.badge(text, color="secondary", text_color="dark").classes("text-[10px]")
return ui.badge(text).classes("text-[10px] ui-badge-secondary")
@@ -24,17 +24,12 @@ def render_document_panzoom(*, source: Source) -> None:
document_url = _document_url(source)
document_kind = _document_kind(source)
with ui.card().classes("w-full q-pa-md vibe-card"):
with ui.card().classes("w-full q-pa-md ui-card-surface"):
with ui.row().classes("w-full items-center justify-between no-wrap"):
ui.label("Document preview").classes("text-subtitle1 text-weight-medium")
ui.label(source.filename).classes("text-caption vibe-text-muted ellipsis").style(
"max-width: 60%; text-align: right;"
)
ui.label(source.filename).classes("text-caption ui-text-muted ellipsis document-panzoom-filename")
with (
ui.element("div").classes("w-full document-panzoom-host rounded-borders q-mt-md")
# .style(f"height: {height};")
) as host:
with ui.element("div").classes("w-full document-panzoom-host q-mt-md") as host:
host.props(f"id={host_id}")
with ui.element("div").classes("document-panzoom-surface"):
if document_kind == "pdf":
@@ -59,43 +54,6 @@ def _register_panzoom_assets() -> None:
f'<script src="{PANGOZOOM_CDN_URL}"></script>',
shared=True,
)
ui.add_head_html(
"""
<style>
.document-panzoom-host {
overflow: hidden;
touch-action: none;
}
.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);
}
</style>
""",
shared=True,
)
def _document_url(source: Source) -> str:
@@ -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")
@@ -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
return table
@@ -85,15 +85,14 @@ def render_documents_table(rows: Sequence[DocumentTableRow]) -> None:
"body-cell-document_type",
r"""
<q-td :props="props">
<q-chip
dense
square
<q-chip
dense
square
size="sm"
color="primary"
text-color="white"
class="ui-chip-primary"
>
{{ props.value }}
</q-chip>
</q-td>
""",
)
)
+8 -12
View File
@@ -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"""
<q-td :props="props">
<q-chip
dense
square
<q-chip
dense
square
size="sm"
:color="
props.value === 'completed' || props.value === 'transcribed' ? 'positive' :
props.value === 'failed' ? 'negative' :
props.value === 'processing' ? 'secondary' :
props.value === 'queued' ? 'warning' : 'grey-6'
"
text-color="white"
:class="`ui-status ui-status--${props.value}`"
>
{{ props.value.toUpperCase() }}
</q-chip>
</q-td>
""",
)
)
@@ -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"""
<q-td :props="props">
<q-chip
dense
square
<q-chip
dense
square
size="sm"
:color="props.value === 'transcribed' ? 'positive' : props.value === 'failed' ? 'negative' : 'grey-5'"
text-color="white"
:class="`ui-status ui-status--${props.value}`"
>
{{ props.value }}
</q-chip>
</q-td>
""",
)
)
+9 -10
View File
@@ -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")
+5 -6
View File
@@ -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")
"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")
+105 -56
View File
@@ -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;
+45
View File
@@ -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",
"<style": "embedded style block",
"vibe-": "legacy presentation class",
}
violations: list[str] = []
for path in sorted((*UI_ROOT.glob("components/**/*.py"), *UI_ROOT.glob("pages/**/*.py"))):
source = path.read_text(encoding="utf-8")
for pattern, description in prohibited_patterns.items():
if pattern in source:
violations.append(f"{path.relative_to(UI_ROOT)}: {description}")
assert violations == []
@pytest.mark.unit
def test_theme_defines_shared_semantic_surfaces():
theme_css = read_css("theme.css")
for class_name in (
"ui-card-surface",
"ui-card-error",
"ui-form-surface",
"ui-table",
"ui-chip-primary",
"ui-badge-secondary",
"ui-status",
"document-panzoom-host",
):
assert f".{class_name}" in theme_css