refactor(ui): extract _paintProjectPicker (dedupe modal/dashboard)

Review of #868 flagged the sync-paint + refresh + required/optional hint block as copy-pasted between showNewWsModal and _loadDashboardOptionsLists, already diverging structurally, so a future tweak could drift and silently re-introduce the FOUC on the missed surface. Collapse both into a shared _paintProjectPicker(sel, hint, {fork}) -- the modal passes the fork flag, the dashboard never forks. Guards re-pointed at the helper + a new one pins its sync-before-async pattern.
This commit is contained in:
Patrick Buckley
2026-07-18 17:11:41 -07:00
parent 4079542447
commit 079257967d
2 changed files with 56 additions and 69 deletions
+30 -19
View File
@@ -2407,15 +2407,10 @@ def test_new_ws_modal_paints_project_and_persona_from_cache_synchronously() -> N
these guards pin only that a synchronous populate precedes it."""
body = _APP_JS.read_text(encoding="utf-8")
fn = _slice_top_level_fn(body, "function showNewWsModal(")
# The FIRST _populateProjectSelect(projSelect, …) is the sync paint; it must
# precede the refreshProjects().then(…) async repaint.
sync_proj = fn.find("_populateProjectSelect(projSelect")
async_proj = fn.find("refreshProjects().then")
assert sync_proj >= 0, "the modal must paint the project picker from cache synchronously"
assert async_proj >= 0, "the modal must keep refreshProjects().then (catches remote changes)"
assert sync_proj < async_proj, (
"the synchronous project paint must precede the async refreshProjects().then "
"repaint — otherwise the picker flashes empty on every open"
# Project is painted via the shared _paintProjectPicker helper (fork-gated);
# the sync-before-async pattern is pinned in test_paint_project_picker_syncs.
assert "_paintProjectPicker(projSelect" in fn, (
"the modal must paint the project picker via the shared _paintProjectPicker helper"
)
sync_persona = fn.find("_populatePersonaSelect(personaSelect")
async_persona = fn.find("refreshPersonas().then")
@@ -2434,12 +2429,9 @@ def test_dashboard_paints_project_and_persona_from_cache_synchronously() -> None
from requireProject() synchronously, matching the new-ws modal."""
body = _APP_JS.read_text(encoding="utf-8")
fn = _slice_top_level_fn(body, "function _loadDashboardOptionsLists(")
sync_proj = fn.find("_populateProjectSelect(projSel")
async_proj = fn.find("refreshProjects().then")
assert sync_proj >= 0, "the dashboard must paint the project picker from cache synchronously"
assert async_proj >= 0, "the dashboard must keep refreshProjects().then"
assert sync_proj < async_proj, (
"the synchronous dashboard project paint must precede the async refresh repaint"
# Project is painted via the shared _paintProjectPicker helper.
assert "_paintProjectPicker(projSel" in fn, (
"the dashboard must paint the project picker via the shared _paintProjectPicker helper"
)
sync_persona = fn.find("_populatePersonaSelect(personaSel")
async_persona = fn.find("refreshPersonas().then")
@@ -2455,10 +2447,7 @@ def test_dashboard_paints_project_and_persona_from_cache_synchronously() -> None
assert 'class="label-hint"' in html[lbl : lbl + 120], (
"the dashboard Project label must carry a .label-hint span (required/optional cue)"
)
sync_hint = fn.find("projHint.textContent")
assert 0 <= sync_hint < async_proj, (
"the dashboard project label hint must be seeded synchronously (before the refresh)"
)
# The hint is seeded synchronously inside _paintProjectPicker (asserted there).
def test_console_launcher_paints_project_and_persona_from_cache_synchronously() -> None:
@@ -2485,3 +2474,25 @@ def test_console_launcher_paints_project_and_persona_from_cache_synchronously()
assert sync_persona < async_persona, (
"the synchronous launcher persona paint must precede the async refresh repaint"
)
def test_paint_project_picker_syncs_before_refresh() -> None:
"""The shared _paintProjectPicker (used by BOTH the modal and dashboard, so
the two can't drift and silently re-introduce the FOUC) seeds the required/
optional hint + paints the picker via _populateProjectSelect SYNCHRONOUSLY,
then refreshes-and-repaints. It skips a fork, and both paints route through
the same _populateProjectSelect (preserving the #867 strict-picker invariant)."""
body = _APP_JS.read_text(encoding="utf-8")
fn = _slice_top_level_fn(body, "function _paintProjectPicker(")
assert "_populateProjectSelect(" in fn, (
"_paintProjectPicker must paint via _populateProjectSelect"
)
assert "hint.textContent" in fn, "_paintProjectPicker must seed the required/optional hint"
assert "opts.fork" in fn, "_paintProjectPicker must skip for a fork"
sync_call = fn.find("paint();")
async_refresh = fn.find("refreshProjects().then")
assert async_refresh >= 0, "_paintProjectPicker must keep refreshProjects().then"
assert 0 <= sync_call < async_refresh, (
"_paintProjectPicker must paint synchronously (paint()) BEFORE the async "
"refreshProjects().then repaint"
)
+26 -50
View File
@@ -414,33 +414,11 @@ function showNewWsModal(forkFromWsId) {
const projHint = projLabel ? projLabel.querySelector(".label-hint") : null;
if (projLabel) projLabel.hidden = !!_forkFromWsId;
if (projSelect) projSelect.hidden = !!_forkFromWsId;
// Seed the hint from the warm cache SYNCHRONOUSLY so a fresh create isn't
// mislabeled "optional" for the duration of the (redundant) refresh round-trip
// — requireProject() reads a cache the rail warms at startup. The refresh below
// re-affirms it for the rare cold-cache open.
if (projHint && !_forkFromWsId && window.TurnstoneProjects) {
projHint.textContent = window.TurnstoneProjects.requireProject()
? "required"
: "optional";
}
if (projSelect && !_forkFromWsId && window.TurnstoneProjects) {
// Paint from the warm cache SYNCHRONOUSLY first so the picker isn't empty for
// the (redundant) refresh round-trip — the rail warms projects at startup and
// projectChoices()/requireProject() are sync. On a cold cache projectChoices()
// is [] and this is a no-op the async fills, so it's never worse than before.
// BOTH paints route through the SAME _populateProjectSelect, so the #867
// strict-picker invariant (never auto-select a real project) holds identically.
_populateProjectSelect(projSelect, {
requireProject: !!window.TurnstoneProjects.requireProject(),
});
window.TurnstoneProjects.refreshProjects().then(function () {
const strict = !!window.TurnstoneProjects.requireProject();
// Honest label: under require_project a fresh create must resolve to a real
// project, so the static "optional" hint must not claim otherwise.
if (projHint) projHint.textContent = strict ? "required" : "optional";
_populateProjectSelect(projSelect, { requireProject: strict });
});
}
// Paint the picker + its required/optional hint from the warm cache, then
// refresh-and-repaint — shared with the dashboard via _paintProjectPicker so
// the two can't drift; skips for a fork (its picker is hidden above,
// inheritance is server-enforced).
_paintProjectPicker(projSelect, projHint, { fork: !!_forkFromWsId });
// Persona picker — hidden when forking (a fork resumes the source's
// stamped persona; the create handler skips resolution on resume_ws).
@@ -571,6 +549,24 @@ function _optionExists(sel, val) {
return false;
}
// Paint a FRESH-create project picker (+ its required/optional label hint) from
// the warm cache SYNCHRONOUSLY, then refresh-and-repaint. Shared by the new-ws
// modal and the dashboard composer so the two paths can't drift and silently
// re-introduce the empty-dropdown / mislabel FOUC this exists to prevent. A fork
// skips entirely (a fork inherits its source's project server-side; the modal
// hides the picker for forks). Both paints reuse _populateProjectSelect, so the
// #867 strict-picker invariant (never auto-select a real project) holds on each.
function _paintProjectPicker(sel, hint, opts) {
if ((opts && opts.fork) || !sel || !window.TurnstoneProjects) return;
const paint = function () {
const strict = !!window.TurnstoneProjects.requireProject();
if (hint) hint.textContent = strict ? "required" : "optional";
_populateProjectSelect(sel, { requireProject: strict });
};
paint(); // sync from the warm cache (no-op when cold; the async fills it)
window.TurnstoneProjects.refreshProjects().then(paint);
}
// Fill a project <select> from the shared projects cache. The picker MODE is
// passed EXPLICITLY by each caller as {requireProject}; a re-populate from the
// inline "+ New project…" creator passes nothing and reuses the mode stamped on
@@ -1513,32 +1509,12 @@ function _loadDashboardOptionsLists() {
}
// Project picker — paint from the warm cache then refresh-and-repaint (also
// feeds the rail's group-by-project). Re-fetched each time the options open so
// a project created elsewhere appears without a page reload.
// feeds the rail's group-by-project). Dashboard quick-create is ALWAYS a fresh
// create (never a fork); shared with the modal via _paintProjectPicker.
const projSel = document.getElementById("dashboard-project");
const projLabel = document.querySelector('label[for="dashboard-project"]');
const projHint = projLabel ? projLabel.querySelector(".label-hint") : null;
if (projSel && window.TurnstoneProjects) {
// Seed the "required"/"optional" label hint + paint the picker from the warm
// cache SYNCHRONOUSLY (mirrors showNewWsModal) so a fresh open isn't empty or
// mislabeled for the refresh round-trip; the async refresh below re-affirms
// both to catch a project created elsewhere. Dashboard quick-create is ALWAYS
// a fresh create (never a fork); the mode is passed explicitly so the shared
// helper never reads the fork-only global.
if (projHint) {
projHint.textContent = window.TurnstoneProjects.requireProject()
? "required"
: "optional";
}
_populateProjectSelect(projSel, {
requireProject: !!window.TurnstoneProjects.requireProject(),
});
window.TurnstoneProjects.refreshProjects().then(function () {
const strict = !!window.TurnstoneProjects.requireProject();
if (projHint) projHint.textContent = strict ? "required" : "optional";
_populateProjectSelect(projSel, { requireProject: strict });
});
}
_paintProjectPicker(projSel, projHint, { fork: false });
// Persona picker — same paint-from-cache-then-refresh policy; kind default
// preselected so a zero-touch launch behaves exactly like today.