phase: 156_table_ux_polish
Everything is verified. All gates green on the current working tree. Final report: **Phase 156 — final verification pass (all 4 task files present in `complete/` — no outstanding work)** - **Verified (not re-implemented):** bounded-panel `.table-wrap` (`max-height: min(70vh, 64rem)`, `overflow: auto`, opaque sticky `th` via `var(--brand-soft)`), `.cell-clip` ellipsis rule + the five label families in the three renderers, the added aria-labels, unit pins (incl. `.md-table-wrap` byte-unchanged scope guard, `#git-sources-table-wrap` harmony — no divergent rule), dedicated E2E suite (sticky ×3 tables, horizontal contract, computed-style ellipsis + reachable full text, mobile hamburger + scroll + pin) - **Tests/lint/coverage:** `uv run pytest --cov=app --cov-report=term-missing` → **3570 passed, 99% on `app/`** (gate >90%); `uv run pytest tests/e2e/test_table_ux_polish.py -v --no-cov` (DB up) → **6/6 passed in isolation**; `uv run ruff check . && uv run pyright` → **clean** (0 errors) - **Completion criteria:** sticky header all 3 tables (exact-0 offset while `scrollTop > 0`) ✅ · horizontal scroll unchanged ✅ · ellipsis computed style + actual clip + title/aria reachable ✅ · mobile ≤640px contract ✅ · full suite/E2E/lint/coverage ✅ · atomic commit: **harness commits this phase after the pass** (per pipeline protocol — no git history commands run by me) - **Documentation:** AGENTS.md phase-156 wording (bounded panels, sticky headers, `.cell-clip` + aria-label idiom) matches the implementation exactly — verified consistent; README.md reviewed in full (pages/URLs vs registered routes, all `.env.example` vars documented, Quick Start, eval/llm scripts, compose prod profile, E2E examples) — verified consistent, no changes needed (README describes no table layout/scroll, so no phase-156 mention required) - **Deviations:** none — all work was already in the working tree; this pass re-verified every gate and the docs - **Next pending phase:** `157_chat_chip_cap`
This commit is contained in:
@@ -19,6 +19,16 @@ DB-over-built-in resolver shared by ``/api/ui-settings`` and
|
||||
(the resolver treats "" as unset, B1). House DB-test pattern (the
|
||||
``test_tokens`` precedent): the real compose Postgres, skipped with
|
||||
clear instructions when the stack is not up.
|
||||
* Phase 156 (task 03): the catalogue-table CSS contract pins, parsed
|
||||
straight out of ``styles.css`` (the :root-drift idiom extended — a
|
||||
later "cleanup" cannot silently unstick the header, unbound the
|
||||
panel, or transparent the thead): the bounded ``.table-wrap``
|
||||
panel, the sticky opaque ``thead th``, the
|
||||
``#git-sources-table-wrap`` one-rule harmony, the
|
||||
``.md-table-wrap`` byte-unchanged scope guard, the ``.cell-clip``
|
||||
ellipsis. The :root 17-variable set itself stays pinned by
|
||||
``test_builtin_colors_match_styles_css_root`` (UNCHANGED — this
|
||||
phase adds no palette variable, B3).
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
@@ -389,3 +399,176 @@ def test_docstring_pair_table_matches_theme_js_pairs() -> None:
|
||||
# surface in BOTH places (decorative borders — no contrast duty).
|
||||
assert "err_line" not in [f for f, _ in pairs]
|
||||
assert "accent_line" not in [f for f, _ in pairs]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Phase 156 (task 03): the catalogue-table CSS contract pins — the
|
||||
# bounded-panel sticky header + the clipped-label ellipsis, parsed
|
||||
# straight out of styles.css (the :root-drift idiom extended: a later
|
||||
# "cleanup" cannot silently unstick the header, unbound the panel, or
|
||||
# transparent the thead). The :root 17-variable set itself is pinned
|
||||
# by test_builtin_colors_match_styles_css_root above (UNCHANGED —
|
||||
# this phase adds no palette variable, B3: the 17-variable set stays
|
||||
# byte-identical).
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _css_rule(selector: str) -> dict[str, str] | None:
|
||||
"""The ``prop: value`` declarations (comments stripped) of the FIRST
|
||||
styles.css rule whose selector ends with ``selector`` (a bare
|
||||
``.table-wrap`` never matches ``.table-wrap table thead th`` —
|
||||
the selector must end right at the rule's ``{``), or ``None``
|
||||
when the selector is absent from the file."""
|
||||
css = re.sub(r"/\*.*?\*/", "", STYLES_CSS.read_text(encoding="utf-8"), flags=re.S)
|
||||
match = re.search(re.escape(selector) + r"(?![\w.#-])\s*\{", css)
|
||||
if match is None:
|
||||
return None
|
||||
block = css[match.end() : css.index("}", match.end())]
|
||||
return dict(re.findall(r"([a-z-]+)\s*:\s*([^;]+);", block))
|
||||
|
||||
|
||||
def _assert_opaque_background(value: str, root: dict[str, str]) -> None:
|
||||
"""The sticky th's OPACITY pin (the exact token is the phase's
|
||||
choice — ``var(--brand-soft)`` today; the opacity is the pin):
|
||||
the background token must paint a solid, alpha-less color —
|
||||
``transparent``/``none`` never, a ``var(--x)`` token must resolve
|
||||
to a solid six-hex ``:root`` value, a literal must carry no
|
||||
alpha channel (``rgba(`` / ``hsla(`` / 8-digit hex all let rows
|
||||
show through the pinned header)."""
|
||||
v = value.strip()
|
||||
assert v.lower() not in {"transparent", "none", "inherit", "unset", "initial"}, (
|
||||
f"the sticky th background is {value!r} — rows would show through"
|
||||
)
|
||||
var = re.fullmatch(r"var\(\s*(--[a-z-]+)\s*\)", v)
|
||||
if var:
|
||||
name = var.group(1)
|
||||
assert name in root, f"the sticky th background {value!r} names an unknown {name}"
|
||||
resolved = root[name]
|
||||
assert re.fullmatch(r"#[0-9a-f]{6}", resolved), (
|
||||
f"the sticky th background {value!r} resolves to {resolved!r} — it must "
|
||||
f"be a solid alpha-less hex (OPAQUE)"
|
||||
)
|
||||
return
|
||||
assert not re.search(r"rgba?\(\s|hsla?\(\s|#[0-9a-f]{8}\b", v), (
|
||||
f"the sticky th background {value!r} carries an alpha channel — "
|
||||
f"it must be OPAQUE"
|
||||
)
|
||||
|
||||
|
||||
def test_table_wrap_bounded_panel_pin() -> None:
|
||||
"""The ONE rule all three catalogue tables share (the RAG
|
||||
``.docs-table``, the Sources ``.git-sources-table``, the History
|
||||
``.history-table``): the card is the vertical scroll region —
|
||||
``max-height: min(70vh, 64rem)`` (the exact A2 value pinned: a
|
||||
15-row table scrolls while a 3-row one fits without a scrollbar)
|
||||
+ ``overflow: auto`` (both axes — the tables' min-width still
|
||||
triggers the in-card horizontal scroll at narrow widths). The
|
||||
pre-phase ``overflow-x: auto`` is SUPERSEDED — its return would
|
||||
unbound the panel's vertical contract (a sticky header in an
|
||||
unbounded card scrolls away with the page)."""
|
||||
rule = _css_rule(".table-wrap")
|
||||
assert rule is not None, "styles.css must define .table-wrap"
|
||||
assert rule.get("max-height") == "min(70vh, 64rem)", (
|
||||
f".table-wrap max-height drifted: {rule.get('max-height')!r} — the "
|
||||
f"bounded-panel pin is min(70vh, 64rem)"
|
||||
)
|
||||
assert rule.get("overflow") == "auto", (
|
||||
f".table-wrap overflow drifted: {rule.get('overflow')!r} — the card "
|
||||
f"must be the sticky th's scrolling ancestor (overflow: auto)"
|
||||
)
|
||||
assert "overflow-x" not in rule, (
|
||||
".table-wrap must not carry a bare overflow-x — the pre-phase "
|
||||
"overflow-x: auto is superseded by overflow: auto (both axes)"
|
||||
)
|
||||
|
||||
|
||||
def test_table_wrap_thead_sticky_pin() -> None:
|
||||
"""The ONE sticky-header rule for every catalogue table:
|
||||
``position: sticky; top: 0; z-index: 1`` (the header row pinned
|
||||
to the card top while the card's own vertical scroll moves, the
|
||||
pinned th above the scrolled cells) + an OPAQUE background — the
|
||||
parse asserts the background is a non-``transparent`` token (the
|
||||
exact token is the phase's choice — ``var(--brand-soft)`` today;
|
||||
the OPACITY is the pin: a ``var`` token must resolve to a solid
|
||||
alpha-less :root hex, a literal must carry no alpha channel —
|
||||
rows must never show through the pinned header)."""
|
||||
rule = _css_rule(".table-wrap table thead th")
|
||||
assert rule is not None, "styles.css must define .table-wrap table thead th"
|
||||
assert rule.get("position") == "sticky", (
|
||||
f"the catalogue thead th must stay position: sticky, "
|
||||
f"got {rule.get('position')!r}"
|
||||
)
|
||||
assert rule.get("top") == "0", (
|
||||
f"the catalogue thead th must pin at top: 0, got {rule.get('top')!r}"
|
||||
)
|
||||
assert rule.get("z-index") == "1", (
|
||||
f"the catalogue thead th must sit at z-index: 1, got {rule.get('z-index')!r}"
|
||||
)
|
||||
assert "background" in rule, (
|
||||
"the catalogue thead th must carry an explicit background "
|
||||
"(the opaque pin — the default is transparent)"
|
||||
)
|
||||
_assert_opaque_background(rule["background"], _root_declarations())
|
||||
|
||||
|
||||
def test_git_sources_table_wrap_no_divergent_override() -> None:
|
||||
"""The one-rule contract: the Sources card
|
||||
(``#git-sources-table-wrap``) must not carry a DIVERGENT UNBOUNDED
|
||||
override — if the id rule exists it carries the SAME
|
||||
``max-height: min(70vh, 64rem)`` + ``overflow: auto`` as the base
|
||||
``.table-wrap`` (a divergent ``overflow-x: auto`` without the
|
||||
bound would make the Sources table scroll away with the page
|
||||
while the other two stick). The current form (task 01): the id
|
||||
rule is REMOVED — the wrapper carries ``class="table-wrap"`` and
|
||||
inherits the base card's bound + overflow + sticky rule, so the
|
||||
rule's absence IS the harmony."""
|
||||
rule = _css_rule("#git-sources-table-wrap")
|
||||
if rule is None:
|
||||
return # the one-rule contract: the id carries no rule of its own
|
||||
assert rule.get("max-height") == "min(70vh, 64rem)", (
|
||||
f"#git-sources-table-wrap max-height diverges: "
|
||||
f"{rule.get('max-height')!r} — the one-rule contract pins min(70vh, 64rem)"
|
||||
)
|
||||
assert rule.get("overflow") == "auto", (
|
||||
f"#git-sources-table-wrap overflow diverges: {rule.get('overflow')!r} — "
|
||||
f"the one-rule contract pins overflow: auto"
|
||||
)
|
||||
|
||||
|
||||
def test_md_table_wrap_byte_unchanged_pin() -> None:
|
||||
"""The scope guard: the CHAT view's markdown table wrapper
|
||||
(``.md-table-wrap`` — the shared renderer's horizontal scroller)
|
||||
is a DIFFERENT class from the catalogue card — UNTOUCHED by this
|
||||
phase (no card, no height bound; the bounded-panel design is for
|
||||
the catalogue tables only). The pin asserts the rule's VERBATIM
|
||||
pre-phase text (byte-identical)."""
|
||||
css = STYLES_CSS.read_text(encoding="utf-8")
|
||||
assert ".md-table-wrap { overflow-x: auto; }" in css, (
|
||||
".md-table-wrap must stay byte-identical to its pre-phase rule "
|
||||
"(.md-table-wrap { overflow-x: auto; }) — the chat scope guard"
|
||||
)
|
||||
|
||||
|
||||
def test_cell_clip_ellipsis_pin() -> None:
|
||||
"""The clipped-label ellipsis (A3): the ``.cell-clip`` label span
|
||||
(the five control families — the Sources Hidden checkbox label +
|
||||
Ignore paths + Remove, the RAG folder table's Edit, the History
|
||||
Copy / Unshare / Create link / Delete) shows "…" instead of the
|
||||
card edge's hard cut: ``text-overflow: ellipsis`` +
|
||||
``overflow: hidden`` + ``white-space: nowrap`` (the house
|
||||
.sync-label shrinkable-item mechanism; the max-width clamp
|
||||
engages only in the clipped case)."""
|
||||
rule = _css_rule(".cell-clip")
|
||||
assert rule is not None, "styles.css must define .cell-clip"
|
||||
assert rule.get("text-overflow") == "ellipsis", (
|
||||
f".cell-clip text-overflow drifted: {rule.get('text-overflow')!r} — "
|
||||
f"the clipped label must show the ellipsis hint"
|
||||
)
|
||||
assert rule.get("overflow") == "hidden", (
|
||||
f".cell-clip overflow drifted: {rule.get('overflow')!r} — "
|
||||
f"the ellipsis needs the hidden overflow"
|
||||
)
|
||||
assert rule.get("white-space") == "nowrap", (
|
||||
f".cell-clip white-space drifted: {rule.get('white-space')!r} — "
|
||||
f"the ellipsis needs the nowrap"
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user