feat(ui): one consistent navbar on every page (TODO.md L3)
This commit is contained in:
@@ -3,9 +3,10 @@
|
||||
The browser behavior is E2E-covered (tests/e2e/test_shared_header.py);
|
||||
here we pin the source-level wiring — the header.js exports, the cached
|
||||
whoami promise, the per-page HTML ids (anonymous-safe hidden-by-default
|
||||
controls), the sign-out binding move out of app.js, the non-chat New
|
||||
Chat bindings, and the viewer-bar CSS — so a silent regression is caught
|
||||
without a browser.
|
||||
controls), the sign-out binding move out of app.js, the SINGLE
|
||||
module-owned New chat binding (phase 34 task 02) + the sign-in
|
||||
?next= rewrite, and the viewer-bar CSS — so a silent regression is
|
||||
catched without a browser.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
@@ -109,17 +110,75 @@ def test_sign_out_binding_lives_in_the_shared_module() -> None:
|
||||
|
||||
|
||||
def test_nav_sources_ships_hidden_on_every_nav_page() -> None:
|
||||
"""Phase 19 UX revision (owner permission 2026-08-23): the Sources
|
||||
nav link is hidden for anonymous — so it SHIPS with the hidden
|
||||
attribute (anonymous-safe default) on every page that has a nav
|
||||
(chat, sources, login)."""
|
||||
for html in (INDEX_HTML, SOURCES_HTML, LOGIN_HTML, TUNING_HTML):
|
||||
"""Phase 19 UX revision (owner permission 2026-08-23), completed on
|
||||
all five pages by phase 34 task 03 (owner confirmation 2026-08-26):
|
||||
the Sources nav link is hidden for anonymous — so it SHIPS with the
|
||||
hidden attribute (anonymous-safe default) on every page (they all
|
||||
carry the nav now, viewer included)."""
|
||||
for html in (INDEX_HTML, SOURCES_HTML, DOCUMENT_HTML, LOGIN_HTML, TUNING_HTML):
|
||||
text = _text(html)
|
||||
assert re.search(r'id="nav-sources"[^>]*\bhidden\b', text), (
|
||||
f"{html.name}: #nav-sources must ship hidden"
|
||||
)
|
||||
|
||||
|
||||
def test_all_five_pages_share_the_header_control_order() -> None:
|
||||
"""Phase 34 task 03 (owner confirmation 2026-08-26): every page
|
||||
ships the IDENTICAL header control inventory in the IDENTICAL
|
||||
order — brand, nav [Chat, Sources, Tuning], #steering-toggle,
|
||||
#sync-btn, #new-chat-btn, Sign in, Sign out — inside the shared
|
||||
.header-inner row (the document viewer's row 1). Only the
|
||||
current-page is-active nav marker and the static ?next= fallback
|
||||
may differ per page (task 05's story E2E pins the rendered
|
||||
result)."""
|
||||
markers = (
|
||||
'class="brand"',
|
||||
'<nav class="app-nav"',
|
||||
'href="/"',
|
||||
'id="nav-sources"',
|
||||
'id="nav-tuning"',
|
||||
'id="steering-toggle"',
|
||||
'id="sync-btn"',
|
||||
'id="new-chat-btn"',
|
||||
'id="sign-in-link"',
|
||||
'id="sign-out-btn"',
|
||||
)
|
||||
for html in (INDEX_HTML, SOURCES_HTML, DOCUMENT_HTML, TUNING_HTML, LOGIN_HTML):
|
||||
text = _text(html)
|
||||
start = text.find('<div class="container header-inner">')
|
||||
assert start != -1, f"{html.name}: missing the shared .header-inner row"
|
||||
region = text[start : text.find("</header>", start)]
|
||||
missing = [m for m in markers if m not in region]
|
||||
assert not missing, f"{html.name}: header controls missing {missing}"
|
||||
for m in markers:
|
||||
assert region.count(m) == 1, f"{html.name}: {m} must appear exactly once"
|
||||
# Same order on every page: each control follows the previous one.
|
||||
pos = -1
|
||||
for m in markers:
|
||||
idx = region.find(m, pos + 1)
|
||||
assert idx > pos, f"{html.name}: {m} out of order in the shared bar"
|
||||
pos = idx
|
||||
|
||||
|
||||
def test_all_five_pages_carry_the_steering_panel() -> None:
|
||||
"""Phase 34 task 03: the #steering-panel section (+ the
|
||||
#steering-announcer live region) ships on every page — after
|
||||
#kb-banner in the chat shell, first child of <main> on the other
|
||||
four pages — ship hidden, driven by assets/header.js."""
|
||||
for html in (INDEX_HTML, SOURCES_HTML, DOCUMENT_HTML, TUNING_HTML, LOGIN_HTML):
|
||||
text = _text(html)
|
||||
tag = re.search(r'<section[^>]*id="steering-panel"[^>]*>', text)
|
||||
assert tag, f"{html.name}: missing the #steering-panel section"
|
||||
assert re.search(r'\bhidden\b', tag.group(0)), "the panel ships hidden"
|
||||
assert 'id="steering-list"' in text
|
||||
assert 'id="steering-empty"' in text
|
||||
assert re.search(r'<p[^>]*id="steering-announcer"[^>]*role="status"[^>]*>', text), (
|
||||
f"{html.name}: missing the #steering-announcer live region"
|
||||
)
|
||||
# The announcer follows the panel (the copied index.html block).
|
||||
assert text.find('id="steering-panel"') < text.find('id="steering-announcer"')
|
||||
|
||||
|
||||
def test_nav_tuning_ships_hidden_on_the_tuning_page() -> None:
|
||||
"""Phase 27: the Global Tuning page reuses the shared header — the
|
||||
"Tuning" nav link is admin-only, so it SHIPS hidden (revealed by
|
||||
@@ -139,10 +198,38 @@ def test_nav_tuning_ships_hidden_on_the_tuning_page() -> None:
|
||||
assert [s for s in srcs if "tuning.js" in s]
|
||||
|
||||
|
||||
def test_nav_sources_is_absent_from_the_viewer() -> None:
|
||||
"""The document viewer has no nav — no #nav-sources element there (the
|
||||
module's missing-element no-op keeps it out)."""
|
||||
assert 'id="nav-sources"' not in _text(DOCUMENT_HTML)
|
||||
def test_viewer_carries_the_standard_nav() -> None:
|
||||
"""Phase 34 task 03 (owner confirmation 2026-08-26): the document
|
||||
viewer carries the SAME standard bar as every other page — row 1 is
|
||||
the shared .header-inner block, so the nav (Chat + the admin-only
|
||||
#nav-sources / #nav-tuning links, ship hidden) is there too. No nav
|
||||
link is "current" on the viewer: a document is a detail view
|
||||
reachable from chat or Sources, and the phase-13 back link (row 2)
|
||||
carries the return affordance. The phase-19 single-row bar
|
||||
(.doc-header-actions) is superseded by the two-row layout —
|
||||
.doc-titlebar keeps #doc-back / #doc-title / #doc-meta, so
|
||||
document.js needs no render change."""
|
||||
text = _text(DOCUMENT_HTML)
|
||||
chat = re.search(r'<a[^>]*href="/"[^>]*>Chat</a>', text)
|
||||
assert chat, "the viewer bar carries the standard nav"
|
||||
assert "is-active" not in chat.group(0), "no nav link is current on the viewer"
|
||||
for link in ("nav-sources", "nav-tuning"):
|
||||
tag = re.search(rf'<a[^>]*id="{link}"[^>]*>', text)
|
||||
assert tag, f"the viewer bar must carry #{link}"
|
||||
assert "hidden" in tag.group(0), f"#{link} must ship hidden (admin-only)"
|
||||
assert "is-active" not in tag.group(0)
|
||||
# Row 1 is the standard bar inside the two-row viewer header.
|
||||
assert re.search(r'<header[^>]*class="doc-header"', text), "the header keeps .doc-header"
|
||||
assert 'class="app-header"' in text, "row 1 reuses the .app-header bar"
|
||||
assert 'doc-header-inner' not in text, "the old single-row wrapper is gone"
|
||||
assert 'doc-header-actions' not in text, "the old actions wrapper is gone"
|
||||
# Row 2: the .doc-titlebar keeps the back link + title + meta.
|
||||
assert 'class="doc-titlebar"' in text, "row 2 must be the .doc-titlebar"
|
||||
assert 'id="doc-back"' in text
|
||||
assert 'id="doc-title"' in text
|
||||
assert 'id="doc-meta"' in text
|
||||
back = re.search(r'<a[^>]*id="doc-back"[^>]*href="/sources.html"', text)
|
||||
assert back, "the back link keeps its /sources.html no-JS fallback"
|
||||
|
||||
|
||||
def test_sources_and_viewer_carry_the_shared_controls() -> None:
|
||||
@@ -197,14 +284,31 @@ def test_header_module_loads_before_the_page_script() -> None:
|
||||
)
|
||||
|
||||
|
||||
def test_login_page_carries_no_chat_controls() -> None:
|
||||
"""Noted boundary (owner-confirmed): the login page is the auth page,
|
||||
not an app page — no New Chat / Sign in / Sign out controls there;
|
||||
header.js only toggles the Sources link."""
|
||||
def test_login_page_carries_the_full_header() -> None:
|
||||
"""Phase 34 task 03 (owner confirmation 2026-08-26): the noted
|
||||
boundary is reversed by owner decision — the login page finally
|
||||
carries the FULL shared header: the nav gains the #nav-tuning link
|
||||
(same ship-hidden markup as the other pages), plus the Tuning
|
||||
toggle, the admin-only Sync button, New chat, and the Sign in /
|
||||
Sign out pair (ship hidden — initSharedHeader reveals exactly one
|
||||
after whoami; the static ?next= fallback is the login page itself).
|
||||
No nav link is "current" on the auth page."""
|
||||
text = _text(LOGIN_HTML)
|
||||
assert "new-chat-btn" not in text
|
||||
assert "sign-in-link" not in text
|
||||
assert "sign-out-btn" not in text
|
||||
for marker in (
|
||||
'id="nav-sources"',
|
||||
'id="nav-tuning"',
|
||||
'id="steering-toggle"',
|
||||
'id="sync-btn"',
|
||||
'id="new-chat-btn"',
|
||||
'id="sign-in-link"',
|
||||
'id="sign-out-btn"',
|
||||
):
|
||||
assert marker in text, f"login.html must carry {marker} (phase 34 full header)"
|
||||
assert 'href="/login.html?next=/login.html"' in text, (
|
||||
"the login page's Sign in returns to the login page (no-JS fallback)"
|
||||
)
|
||||
for tag in re.findall(r'<a[^>]*class="nav-link[^"]*"[^>]*>', text):
|
||||
assert "is-active" not in tag, "no nav link is current on the login page"
|
||||
|
||||
|
||||
# ---------- page-script adaptations ----------
|
||||
@@ -249,40 +353,206 @@ def test_login_js_uses_the_shared_fetch_is_admin() -> None:
|
||||
assert "window.location.replace(safeNext())" in js
|
||||
|
||||
|
||||
def test_non_chat_pages_bind_new_chat_to_the_chat_page() -> None:
|
||||
"""On sources, the viewer, and the tuning page, New Chat means "go to
|
||||
the chat, fresh": the binding clears the phase-14 key
|
||||
(clearChatStorage) and navigates to "/" — and each page runs
|
||||
initSharedHeader() at boot on the shared cached whoami. Phase 23:
|
||||
def test_new_chat_binding_is_single_and_module_owned() -> None:
|
||||
"""Phase 34 task 02: header.js owns the SINGLE #new-chat-btn binding
|
||||
(module import, like the sign-out binding): on the chat page
|
||||
(#messages exists) it dispatches window "bor:new-chat" — app.js acts
|
||||
through its own in-flight-turn guard + list reset; on every other
|
||||
page it means "go to the chat, fresh" (clearChatStorage + navigate
|
||||
to "/"). NO page script binds #new-chat-btn anymore, and each page
|
||||
still runs initSharedHeader() on the shared cached whoami. Phase 23:
|
||||
the import is relative (`./header.js`)."""
|
||||
js = _text(HEADER_JS)
|
||||
assert 'querySelector("#new-chat-btn")' in js
|
||||
assert "newChatBtn.addEventListener" in js
|
||||
assert 'querySelector("#messages")' in js, "the chat-page branch key"
|
||||
assert 'new CustomEvent("bor:new-chat")' in js
|
||||
assert "clearChatStorage();" in js
|
||||
assert 'window.location.href = "/"' in js
|
||||
for js_file in (SOURCES_JS, DOCUMENT_JS, TUNING_JS):
|
||||
js = _text(js_file)
|
||||
assert 'from "./header.js"' in js
|
||||
assert "initSharedHeader()" in js
|
||||
btn_idx = js.find("new-chat-btn")
|
||||
clear_idx = js.find("clearChatStorage();")
|
||||
nav_idx = js.find('window.location.href = "/"')
|
||||
assert -1 < btn_idx < clear_idx < nav_idx, (
|
||||
f"{js_file.name}: #new-chat-btn must clear storage then navigate to '/'"
|
||||
page_js = _text(js_file)
|
||||
assert 'from "./header.js"' in page_js
|
||||
assert "initSharedHeader()" in page_js
|
||||
assert "new-chat-btn" not in page_js, (
|
||||
f"{js_file.name}: no #new-chat-btn binding (the module owns it)"
|
||||
)
|
||||
assert 'fetch("/api/whoami")' not in js, (
|
||||
assert 'fetch("/api/whoami")' not in page_js, (
|
||||
f"{js_file.name}: whoami goes through the shared cached promise"
|
||||
)
|
||||
|
||||
|
||||
# ---------- viewer-bar CSS ----------
|
||||
|
||||
|
||||
def test_viewer_bar_css_pushes_actions_right_and_title_clips() -> None:
|
||||
"""styles.css defines .doc-header-actions (margin-left:auto flex
|
||||
cluster) and the title block keeps min-width: 0 so
|
||||
#doc-title/#doc-meta clip instead of overflowing the --header-h bar."""
|
||||
css = _text(STYLES_CSS)
|
||||
block = re.search(r"\.doc-header-actions\s*\{([^}]*)\}", css)
|
||||
assert block, "styles.css must define .doc-header-actions"
|
||||
body = block.group(1)
|
||||
assert "margin-left: auto" in body
|
||||
assert "display: flex" in body
|
||||
assert re.search(r"\.doc-title-block\s*\{[^}]*min-width:\s*0", css), (
|
||||
"the title block must keep clipping while the pills fit"
|
||||
app_js = _text(APP_JS)
|
||||
assert 'window.addEventListener("bor:new-chat", startNewChat)' in app_js
|
||||
assert "newChatBtn.addEventListener" not in app_js, (
|
||||
"app.js acts off the module's event, not its own binding"
|
||||
)
|
||||
|
||||
|
||||
def test_init_shared_header_rewrites_sign_in_next_to_current_page() -> None:
|
||||
"""Phase 34 task 02: initSharedHeader points #sign-in-link at
|
||||
/login.html?next=<current pathname> (default "/") — the admin lands
|
||||
back on the page they signed in from. The page markup keeps its own
|
||||
static ?next= as the no-JS fallback."""
|
||||
js = _text(HEADER_JS)
|
||||
fn = js.find("function initSharedHeader")
|
||||
assert fn != -1
|
||||
body = js[fn : js.find("\n}", fn)]
|
||||
# raw pathname (always a query-safe "/…" string — never "//", and ?
|
||||
# # / spaces stay percent-encoded inside it; login.js safeNext
|
||||
# re-validates), same shape as the static markup fallbacks
|
||||
assert '"/login.html?next=" + (window.location.pathname || "/")' in body
|
||||
|
||||
|
||||
# ---------- phase 34 task 01: the steering controls move to the module ----------
|
||||
# (task 02 — the sync machine + New chat + sign-in next — is pinned in
|
||||
# test_sync_button.py / above)
|
||||
|
||||
|
||||
def test_header_module_owns_the_steering_panel() -> None:
|
||||
"""header.js owns the steering panel behavior (moved from app.js in
|
||||
phase 34 task 01): the module-level null-safe element refs, the
|
||||
newest-first textContent render (XSS contract), the labeled
|
||||
per-note delete, the count badge, the announcer, and the toggle
|
||||
binding that runs at module import (like the sign-out binding)."""
|
||||
js = _text(HEADER_JS)
|
||||
for selector in (
|
||||
"#steering-toggle",
|
||||
"#steering-count",
|
||||
"#steering-panel",
|
||||
"#steering-list",
|
||||
"#steering-empty",
|
||||
"#steering-announcer",
|
||||
):
|
||||
assert f'querySelector("{selector}")' in js, f"missing {selector} ref"
|
||||
assert "function renderSteeringPanel" in js
|
||||
assert "text.className = \"steering-note-text\"" in js
|
||||
assert "text.textContent = n.note" in js, (
|
||||
"XSS contract: the note renders via textContent, never innerHTML"
|
||||
)
|
||||
assert 'del.setAttribute("aria-label", `Delete tuning note: ${n.note}`)' in js
|
||||
assert "async function deleteSteeringNote" in js
|
||||
assert 'fetch(`/api/steering/${encodeURIComponent(id)}`, { method: "DELETE" })' in js
|
||||
assert "steeringToggle.addEventListener" in js, "the toggle binding is module-owned"
|
||||
assert 'setAttribute("aria-expanded"' in js
|
||||
assert "refreshSteering()" in js # re-open refreshes the list
|
||||
|
||||
|
||||
def test_header_module_exports_refresh_and_announce_steering() -> None:
|
||||
"""refreshSteering() (fetch + render; non-2xx / unreachable API →
|
||||
the empty list state) and announceSteering() (the polite live
|
||||
region) are exported for the chat page's per-bubble Tune form.
|
||||
"""
|
||||
js = _text(HEADER_JS)
|
||||
assert "export async function refreshSteering" in js
|
||||
fn = js.find("function refreshSteering")
|
||||
assert fn != -1
|
||||
body = js[fn : js.find("\n}", fn)]
|
||||
assert 'fetch("/api/steering")' in body
|
||||
assert "renderSteeringPanel(notes)" in body
|
||||
assert "export function announceSteering" in js
|
||||
ann = js.find("function announceSteering")
|
||||
assert ann != -1
|
||||
ann_body = js[ann : js.find("\n}", ann)]
|
||||
assert "steeringAnnouncer.textContent = message" in ann_body
|
||||
|
||||
|
||||
def test_init_shared_header_gates_the_steering_surface() -> None:
|
||||
"""Inside initSharedHeader: admin → the list refreshes (count badge
|
||||
right before the panel is ever opened; only when the page ships the
|
||||
panel markup); anonymous → the toggle + panel are REMOVED from the
|
||||
DOM (phase-16 'absent, not hidden') and /api/steering is never
|
||||
fetched."""
|
||||
js = _text(HEADER_JS)
|
||||
fn = js.find("function initSharedHeader")
|
||||
assert fn != -1
|
||||
body = js[fn : js.find("\n}", fn)]
|
||||
assert "if (steeringPanel) refreshSteering();" in body
|
||||
assert "steeringToggle?.remove();" in body
|
||||
assert "steeringPanel?.remove();" in body
|
||||
|
||||
|
||||
def test_app_js_no_longer_owns_the_steering_panel() -> None:
|
||||
"""app.js keeps only the chat-specific per-bubble Tune button +
|
||||
inline form: the panel refs + logic are gone (header.js owns them
|
||||
now), and the form's success path awaits the module's
|
||||
refreshSteering() (announcing through the module's
|
||||
announceSteering()). The form's POST /api/steering + error handling
|
||||
stay in app.js, untouched."""
|
||||
js = _text(APP_JS)
|
||||
for gone in (
|
||||
"steeringToggle",
|
||||
"steeringCount",
|
||||
"steeringPanel",
|
||||
"steeringList",
|
||||
"steeringEmpty",
|
||||
"steeringAnnouncer",
|
||||
"loadSteering",
|
||||
"renderSteeringPanel",
|
||||
"deleteSteeringNote",
|
||||
"setSteeringPanel",
|
||||
'querySelector("#steering-toggle")',
|
||||
):
|
||||
assert gone not in js, f"{gone!r} must be gone from app.js (header.js owns it)"
|
||||
# the chat-specific part survives and is wired to the shared module
|
||||
assert "function appendTuneButton" in js
|
||||
assert "function openTuneForm" in js
|
||||
assert "TUNE_ICON" in js
|
||||
assert 'from "./header.js"' in js
|
||||
assert "refreshSteering" in js and "announceSteering" in js
|
||||
assert "await refreshSteering()" in js
|
||||
assert 'fetch("/api/steering", {' in js # the form's POST still lives here
|
||||
|
||||
|
||||
# ---------- viewer two-row-header CSS (phase 34 task 04) ----------
|
||||
|
||||
|
||||
def test_viewer_two_row_header_css() -> None:
|
||||
"""Phase 34 task 04: the viewer header is two rows — row 1 reuses
|
||||
the .app-header / .header-inner rules verbatim (the pinned
|
||||
--header-h height still applies to row 1), row 2 is the
|
||||
.doc-titlebar: a quiet --line border-top separator, a flex
|
||||
.container row, its own content-sized height. The old single-row
|
||||
.doc-header-actions / .doc-header-inner rules are gone, .doc-header
|
||||
no longer pins a fixed height, and .doc-title-block keeps
|
||||
min-width: 0 so #doc-title / #doc-meta ellipsize in the row."""
|
||||
css = _text(STYLES_CSS)
|
||||
block = re.search(r"\.doc-titlebar\s*\{([^}]*)\}", css)
|
||||
assert block, "styles.css must define .doc-titlebar"
|
||||
body = block.group(1)
|
||||
assert "border-top: 1px solid var(--line)" in body, (
|
||||
"the titlebar separates from row 1 with the quiet hairline"
|
||||
)
|
||||
assert "height: var(--header-h)" not in body, (
|
||||
"the titlebar row is content-sized (title line + meta line)"
|
||||
)
|
||||
assert re.search(r"\.doc-titlebar\s*\.container\s*\{[^}]*display:\s*flex", css), (
|
||||
"the titlebar row must be a flex row (back link + title block)"
|
||||
)
|
||||
assert re.search(r"\.doc-header-actions\s*\{", css) is None, (
|
||||
"the old single-row actions cluster rule is gone"
|
||||
)
|
||||
assert re.search(r"\.doc-header-inner\s*\{", css) is None, (
|
||||
"the old single-row wrapper rule is gone"
|
||||
)
|
||||
header_block = re.search(r"\.doc-header\s*\{([^}]*)\}", css)
|
||||
assert header_block, "styles.css must still style the .doc-header header element"
|
||||
assert "height: var(--header-h)" not in header_block.group(1), (
|
||||
"the two-row header is content-sized — row 1 keeps the pinned height"
|
||||
)
|
||||
assert re.search(r"\.doc-title-block\s*\{[^}]*min-width:\s*0", css), (
|
||||
"the title block must keep clipping while the row stays tidy"
|
||||
)
|
||||
|
||||
|
||||
def test_failed_sync_button_carries_the_error_look() -> None:
|
||||
"""Phase 34 task 04: the #sync-btn now lives on every page, and the
|
||||
non-Sources pages have no error banner — the failed state must be
|
||||
visible on the button itself. header.js adds .is-error (with the
|
||||
sanitized error in title / aria-label); styles.css must render it
|
||||
with the phase-08 error pair (--err-ink on --err-bg ≈9.1:1,
|
||||
--err-line border — never the amber deflection accent)."""
|
||||
css = _text(STYLES_CSS)
|
||||
block = re.search(r"\.sync-btn\.is-error\s*\{([^}]*)\}", css)
|
||||
assert block, "styles.css must define the failed .sync-btn look"
|
||||
body = block.group(1)
|
||||
assert "var(--err-bg)" in body
|
||||
assert "var(--err-ink)" in body
|
||||
assert "var(--accent-" not in body, "the amber deflection accent is never on errors"
|
||||
|
||||
Reference in New Issue
Block a user