Remove UI window hooks and wire explicit runtime callbacks
This commit is contained in:
14
REVIEW.md
14
REVIEW.md
@@ -8,8 +8,8 @@ Active maintainability risks (priority order):
|
|||||||
|
|
||||||
1. Frontend module concentration and global coupling (Critical)
|
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.
|
- `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`.
|
- 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.
|
- Impact: high regression surface and expensive refactors even after removing global `window` bridges.
|
||||||
|
|
||||||
2. Frontend refresh scheduling can overlap async work (High)
|
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`.
|
- 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
|
## 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.
|
- 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`.
|
- 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 replace `window.*` callbacks with explicit imports/events.
|
- Recommendation: split by feature (`suggestions-ui`, `votes-ui`, `admin-ui`, `modals-ui`) and keep orchestration in a thin composition layer.
|
||||||
- Acceptance criteria (testable): no feature path depends on `window.refreshPhaseData`, `window.loadVoteData`, or `window.loadSuggestData`.
|
- 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`.
|
- Effort / Risk: `L / Med`.
|
||||||
- Dependencies (if any): none.
|
- Dependencies (if any): none.
|
||||||
|
|
||||||
@@ -79,7 +79,7 @@ Active maintainability risks (priority order):
|
|||||||
|
|
||||||
## C) Suggested execution 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.
|
2. Introduce serialized refresh scheduler.
|
||||||
3. Remove `Reveal` phase compatibility branches.
|
3. Remove `Reveal` phase compatibility branches.
|
||||||
4. Normalize/declare unauthenticated 401 contract behavior.
|
4. Normalize/declare unauthenticated 401 contract behavior.
|
||||||
|
|||||||
@@ -19,6 +19,7 @@ import {
|
|||||||
updatePhaseNav,
|
updatePhaseNav,
|
||||||
openConfirmModal,
|
openConfirmModal,
|
||||||
openResultsRelockModal,
|
openResultsRelockModal,
|
||||||
|
configureUiRuntime,
|
||||||
} from "./js/ui.js";
|
} from "./js/ui.js";
|
||||||
import {
|
import {
|
||||||
loadState,
|
loadState,
|
||||||
@@ -29,6 +30,12 @@ import {
|
|||||||
refreshPhaseData,
|
refreshPhaseData,
|
||||||
} from "./js/data.js";
|
} from "./js/data.js";
|
||||||
initI18n();
|
initI18n();
|
||||||
|
configureUiRuntime({
|
||||||
|
refreshPhaseData,
|
||||||
|
loadSuggestData,
|
||||||
|
loadVoteData,
|
||||||
|
handleAuthError: (err) => handleAuthError(err, clearUserState),
|
||||||
|
});
|
||||||
|
|
||||||
function setupHandlers() {
|
function setupHandlers() {
|
||||||
const toggleAuth = $("auth-toggle");
|
const toggleAuth = $("auth-toggle");
|
||||||
|
|||||||
@@ -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);
|
|
||||||
|
|||||||
@@ -33,6 +33,17 @@ const safeUrl = (url) => {
|
|||||||
};
|
};
|
||||||
const cssEscapeUrl = (url) => url.replace(/['")\\]/g, "\\$&");
|
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) {
|
export function setAuthUI(isAuthed) {
|
||||||
const main = document.querySelector("main");
|
const main = document.querySelector("main");
|
||||||
const statusBar = document.querySelector(".status-bar");
|
const statusBar = document.querySelector(".status-bar");
|
||||||
@@ -251,7 +262,7 @@ export function renderVotes() {
|
|||||||
const peerSlider = document.querySelector(`input[type=range][data-id="${id}"]`);
|
const peerSlider = document.querySelector(`input[type=range][data-id="${id}"]`);
|
||||||
if (peerSlider) delete peerSlider.dataset.pending;
|
if (peerSlider) delete peerSlider.dataset.pending;
|
||||||
});
|
});
|
||||||
await window.loadVoteData();
|
await runtime.loadVoteData();
|
||||||
updateMissingBadgeFromDom();
|
updateMissingBadgeFromDom();
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
delete e.target.dataset.pending;
|
delete e.target.dataset.pending;
|
||||||
@@ -470,7 +481,7 @@ export function buildCard(
|
|||||||
await api.updateSuggestion(s.id, data);
|
await api.updateSuggestion(s.id, data);
|
||||||
toast(t("toast.savedChanges"));
|
toast(t("toast.savedChanges"));
|
||||||
close();
|
close();
|
||||||
await window.refreshPhaseData();
|
await runtime.refreshPhaseData();
|
||||||
},
|
},
|
||||||
lockTitle,
|
lockTitle,
|
||||||
}),
|
}),
|
||||||
@@ -658,7 +669,7 @@ function openSuggestionModal({ title, submitLabel, initial = {}, onSubmit, lockT
|
|||||||
try {
|
try {
|
||||||
await onSubmit(data, close, submitBtn);
|
await onSubmit(data, close, submitBtn);
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
if (window.handleAuthError?.(err)) return;
|
if (runtime.handleAuthError?.(err)) return;
|
||||||
toast(err.message, true);
|
toast(err.message, true);
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
@@ -681,9 +692,9 @@ export function openNewSuggestionModal() {
|
|||||||
if (submitBtn) triggerCelebration(submitBtn);
|
if (submitBtn) triggerCelebration(submitBtn);
|
||||||
close();
|
close();
|
||||||
if (wasVotePhase) {
|
if (wasVotePhase) {
|
||||||
await window.refreshPhaseData();
|
await runtime.refreshPhaseData();
|
||||||
} else {
|
} else {
|
||||||
await window.loadSuggestData();
|
await runtime.loadSuggestData();
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
@@ -994,7 +1005,7 @@ function openDeleteConfirmModal(s) {
|
|||||||
await api.deleteSuggestion(s.id);
|
await api.deleteSuggestion(s.id);
|
||||||
toast(t("toast.suggestionDeleted"));
|
toast(t("toast.suggestionDeleted"));
|
||||||
close();
|
close();
|
||||||
await window.loadSuggestData();
|
await runtime.loadSuggestData();
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
toast(err.message, true);
|
toast(err.message, true);
|
||||||
}
|
}
|
||||||
@@ -1084,7 +1095,7 @@ function openUnlinkConfirm(s) {
|
|||||||
await adminApi.unlinkSuggestions(s.id);
|
await adminApi.unlinkSuggestions(s.id);
|
||||||
toast(t("admin.unlinkDone"));
|
toast(t("admin.unlinkDone"));
|
||||||
close();
|
close();
|
||||||
await window.refreshPhaseData();
|
await runtime.refreshPhaseData();
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
toast(err.message, true);
|
toast(err.message, true);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user