diff --git a/REVIEW.md b/REVIEW.md index 627f5d6..4775b1d 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -8,8 +8,8 @@ Active maintainability risks (priority order): 1. Frontend module concentration and global coupling (Critical) - `wwwroot/js/ui.js` remains the dominant hotspot and owns rendering, modal flows, admin flows, and vote logic. -- Hidden module coupling still exists through global bridges in `wwwroot/js/data.js:131`-`wwwroot/js/data.js:134`, with consumers in `wwwroot/js/ui.js:473`, `wwwroot/js/ui.js:684`, and `wwwroot/js/ui.js:997`. -- Impact: high regression surface and expensive refactors. +- Cross-feature coupling still exists via shared mutable state usage across UI/data modules (`wwwroot/js/ui.js:180`, `wwwroot/js/ui.js:401`, `wwwroot/js/ui.js:622`, `wwwroot/js/data.js:82`). +- Impact: high regression surface and expensive refactors even after removing global `window` bridges. 2. Frontend refresh scheduling can overlap async work (High) - Refresh loop is still interval-driven in `wwwroot/app.js:290` with async work rooted in `wwwroot/js/data.js:82`. @@ -29,11 +29,11 @@ Active maintainability risks (priority order): ## B) Active task list -[P1] Decompose frontend UI monolith and remove `window` cross-module hooks +[P1] Decompose frontend UI monolith by feature - Problem: Severity `High`, Category `Architecture/Complexity`. `ui.js` still mixes rendering, form behavior, and mutation flows. -- Evidence: `wwwroot/js/ui.js:390`, `wwwroot/js/ui.js:684`, `wwwroot/js/ui.js:997`, `wwwroot/js/data.js:131`, `wwwroot/js/data.js:134`. -- Recommendation: split by feature (`suggestions-ui`, `votes-ui`, `admin-ui`, `modals-ui`) and replace `window.*` callbacks with explicit imports/events. -- Acceptance criteria (testable): no feature path depends on `window.refreshPhaseData`, `window.loadVoteData`, or `window.loadSuggestData`. +- Evidence: `wwwroot/js/ui.js:180`, `wwwroot/js/ui.js:401`, `wwwroot/js/ui.js:622`, `wwwroot/js/ui.js:903`, `wwwroot/js/data.js:82`. +- Recommendation: split by feature (`suggestions-ui`, `votes-ui`, `admin-ui`, `modals-ui`) and keep orchestration in a thin composition layer. +- Acceptance criteria (testable): UI behavior is preserved while feature modules own isolated responsibilities and no single file coordinates all vote/admin/modal/suggestion interactions. - Effort / Risk: `L / Med`. - Dependencies (if any): none. @@ -79,7 +79,7 @@ Active maintainability risks (priority order): ## C) Suggested execution order -1. Decompose `ui.js` and remove `window` hooks. +1. Decompose `ui.js` by feature and keep orchestration thin. 2. Introduce serialized refresh scheduler. 3. Remove `Reveal` phase compatibility branches. 4. Normalize/declare unauthenticated 401 contract behavior. diff --git a/wwwroot/app.js b/wwwroot/app.js index 26e5f08..8fd73f3 100644 --- a/wwwroot/app.js +++ b/wwwroot/app.js @@ -19,6 +19,7 @@ import { updatePhaseNav, openConfirmModal, openResultsRelockModal, + configureUiRuntime, } from "./js/ui.js"; import { loadState, @@ -29,6 +30,12 @@ import { refreshPhaseData, } from "./js/data.js"; initI18n(); +configureUiRuntime({ + refreshPhaseData, + loadSuggestData, + loadVoteData, + handleAuthError: (err) => handleAuthError(err, clearUserState), +}); function setupHandlers() { const toggleAuth = $("auth-toggle"); diff --git a/wwwroot/js/data.js b/wwwroot/js/data.js index e34202e..34f6d2c 100644 --- a/wwwroot/js/data.js +++ b/wwwroot/js/data.js @@ -126,9 +126,3 @@ export function signatureSuggestions(list) { ]), ); } - -// expose for UI handlers that call back in -window.refreshPhaseData = refreshPhaseData; -window.loadSuggestData = loadSuggestData; -window.loadVoteData = loadVoteData; -window.handleAuthError = (err) => handleAuthError(err, clearUserState); diff --git a/wwwroot/js/ui.js b/wwwroot/js/ui.js index 6ca2c65..a0b0c57 100644 --- a/wwwroot/js/ui.js +++ b/wwwroot/js/ui.js @@ -33,6 +33,17 @@ const safeUrl = (url) => { }; const cssEscapeUrl = (url) => url.replace(/['")\\]/g, "\\$&"); +const runtime = { + refreshPhaseData: async () => { }, + loadSuggestData: async () => { }, + loadVoteData: async () => { }, + handleAuthError: () => false, +}; + +export function configureUiRuntime(deps) { + Object.assign(runtime, deps ?? {}); +} + export function setAuthUI(isAuthed) { const main = document.querySelector("main"); const statusBar = document.querySelector(".status-bar"); @@ -251,7 +262,7 @@ export function renderVotes() { const peerSlider = document.querySelector(`input[type=range][data-id="${id}"]`); if (peerSlider) delete peerSlider.dataset.pending; }); - await window.loadVoteData(); + await runtime.loadVoteData(); updateMissingBadgeFromDom(); } catch (err) { delete e.target.dataset.pending; @@ -470,7 +481,7 @@ export function buildCard( await api.updateSuggestion(s.id, data); toast(t("toast.savedChanges")); close(); - await window.refreshPhaseData(); + await runtime.refreshPhaseData(); }, lockTitle, }), @@ -658,7 +669,7 @@ function openSuggestionModal({ title, submitLabel, initial = {}, onSubmit, lockT try { await onSubmit(data, close, submitBtn); } catch (err) { - if (window.handleAuthError?.(err)) return; + if (runtime.handleAuthError?.(err)) return; toast(err.message, true); } }); @@ -681,9 +692,9 @@ export function openNewSuggestionModal() { if (submitBtn) triggerCelebration(submitBtn); close(); if (wasVotePhase) { - await window.refreshPhaseData(); + await runtime.refreshPhaseData(); } else { - await window.loadSuggestData(); + await runtime.loadSuggestData(); } }, }); @@ -994,7 +1005,7 @@ function openDeleteConfirmModal(s) { await api.deleteSuggestion(s.id); toast(t("toast.suggestionDeleted")); close(); - await window.loadSuggestData(); + await runtime.loadSuggestData(); } catch (err) { toast(err.message, true); } @@ -1084,7 +1095,7 @@ function openUnlinkConfirm(s) { await adminApi.unlinkSuggestions(s.id); toast(t("admin.unlinkDone")); close(); - await window.refreshPhaseData(); + await runtime.refreshPhaseData(); } catch (err) { toast(err.message, true); }