fix: render the view "Back" lands on after the popup is reopened (closes #268)
All checks were successful
check / check (push) Successful in 44s
All checks were successful
check / check (push) Successful in 44s
goBack() only unhid its target. A reopened popup renders the wallet list and the one view restoreView() lands on, so every other view is still the blank static template from index.html: pressing Back from Settings onto an address showed an empty address line and no balances, and the same held for address-token, receive, confirm-tx and transaction. The per-view dispatch and its data guards move out of restoreView() into src/popup/viewRouter.js, and goBack() now routes a popped view through the same code by way of a renderer index.js registers with setBackRenderer(). A view whose backing state is gone falls back to Home the way the restore does, rather than showing an empty template. The renderer declines any view outside RESTORABLE_VIEWS, so goBack() unhides it as before: the restored stack is filtered against that set, so such a view can only be on the stack from the current page load, where forward navigation rendered it on the way in. Forward navigation is untouched and nothing renders twice. tests/backNavigation.test.js drives the real goBack() over the reproduction and over each of address-token, receive, confirm-tx and transaction, with and without their backing state, and pins that a live-session view is still only unhidden. tests/e2e/run.js adds two cases against the real popup in a real browser — a real close and reopen, then Back onto the address screen and onto Receive — because make check cannot see a blank view. Both were demonstrated failing against the unmodified sources: the address line read "" where it should have read the address.
This commit is contained in:
@@ -9,16 +9,17 @@ const {
|
||||
$,
|
||||
showView,
|
||||
updateDebugBanner,
|
||||
setRenderMain,
|
||||
setBackRenderer,
|
||||
pushCurrentView,
|
||||
goBack,
|
||||
clearViewStack,
|
||||
} = require("./views/helpers");
|
||||
const { applyTheme } = require("./theme");
|
||||
// Views that can be fully re-rendered from persisted state. All others fall
|
||||
// back to the nearest restorable parent; see the module for why the
|
||||
// secret-bearing views are absent.
|
||||
const { RESTORABLE_VIEWS } = require("./restorableViews");
|
||||
// Renders a view the popup lands on without having navigated to it forward:
|
||||
// on restore here, and on Back. Only the views that can be fully re-rendered
|
||||
// from persisted state (RESTORABLE_VIEWS, src/popup/restorableViews.js) go
|
||||
// through it; anything else falls back to the nearest restorable parent.
|
||||
const { renderView, makeBackRenderer } = require("./viewRouter");
|
||||
|
||||
const home = require("./views/home");
|
||||
const welcome = require("./views/welcome");
|
||||
@@ -108,91 +109,22 @@ const ctx = {
|
||||
},
|
||||
};
|
||||
|
||||
function needsAddress(view) {
|
||||
return (
|
||||
view === "address" ||
|
||||
view === "address-token" ||
|
||||
view === "receive" ||
|
||||
view === "transaction"
|
||||
);
|
||||
}
|
||||
|
||||
function hasValidAddress() {
|
||||
return (
|
||||
state.selectedWallet !== null &&
|
||||
state.selectedAddress !== null &&
|
||||
state.wallets[state.selectedWallet] &&
|
||||
state.wallets[state.selectedWallet].addresses[state.selectedAddress]
|
||||
);
|
||||
}
|
||||
// The view modules the router renders through, keyed as it expects them.
|
||||
const viewModules = {
|
||||
main: { show: () => fallbackView() },
|
||||
addressDetail,
|
||||
addressToken,
|
||||
receive,
|
||||
settings,
|
||||
settingsAddToken,
|
||||
confirmTx,
|
||||
transactionDetail,
|
||||
txStatus,
|
||||
};
|
||||
|
||||
function restoreView() {
|
||||
const view = state.currentView;
|
||||
if (!view || !RESTORABLE_VIEWS.has(view)) {
|
||||
return fallbackView();
|
||||
}
|
||||
|
||||
if (needsAddress(view) && !hasValidAddress()) {
|
||||
return fallbackView();
|
||||
}
|
||||
|
||||
if (view === "address-token" && !state.selectedToken) {
|
||||
return fallbackView();
|
||||
}
|
||||
|
||||
switch (view) {
|
||||
case "address":
|
||||
addressDetail.show();
|
||||
break;
|
||||
case "address-token":
|
||||
addressToken.show();
|
||||
break;
|
||||
case "receive":
|
||||
receive.show();
|
||||
break;
|
||||
case "settings":
|
||||
settings.show();
|
||||
break;
|
||||
case "settings-addtoken":
|
||||
settingsAddToken.show();
|
||||
break;
|
||||
case "confirm-tx":
|
||||
if (state.viewData && state.viewData.pendingTx) {
|
||||
confirmTx.restore();
|
||||
} else {
|
||||
fallbackView();
|
||||
}
|
||||
break;
|
||||
case "transaction":
|
||||
if (state.viewData && state.viewData.tx) {
|
||||
transactionDetail.render();
|
||||
} else {
|
||||
fallbackView();
|
||||
}
|
||||
break;
|
||||
case "wait-tx":
|
||||
// Resumes the receipt poll from the persisted broadcast time.
|
||||
if (!txStatus.restoreWait()) {
|
||||
fallbackView();
|
||||
}
|
||||
break;
|
||||
case "success-tx":
|
||||
if (state.viewData && state.viewData.hash) {
|
||||
txStatus.renderSuccess();
|
||||
} else {
|
||||
fallbackView();
|
||||
}
|
||||
break;
|
||||
case "error-tx":
|
||||
if (state.viewData && state.viewData.message) {
|
||||
txStatus.renderError();
|
||||
} else {
|
||||
fallbackView();
|
||||
}
|
||||
break;
|
||||
default:
|
||||
fallbackView();
|
||||
break;
|
||||
if (!renderView(state.currentView, state, viewModules)) {
|
||||
fallbackView();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -247,7 +179,7 @@ async function init() {
|
||||
settings.show();
|
||||
});
|
||||
|
||||
setRenderMain(renderWalletList);
|
||||
setBackRenderer(makeBackRenderer(state, viewModules));
|
||||
|
||||
welcome.init(ctx);
|
||||
addWallet.init(ctx);
|
||||
|
||||
116
src/popup/viewRouter.js
Normal file
116
src/popup/viewRouter.js
Normal file
@@ -0,0 +1,116 @@
|
||||
// Rendering a view the popup lands on without having navigated to it
|
||||
// forward: on restore, and on Back. In both cases the view may never have
|
||||
// been rendered in this page load — a reopened popup renders only the
|
||||
// wallet list and the view it restores onto, so every other view is still
|
||||
// the blank static template from index.html — so unhiding it is not enough.
|
||||
//
|
||||
// Forward navigation renders as it goes and must NOT come through here:
|
||||
// rendering a second time would re-fetch and clobber whatever the view has
|
||||
// in flight.
|
||||
//
|
||||
// The view modules are injected and nothing here touches the DOM, so the
|
||||
// dispatch and its data guards can be tested directly; src/popup/index.js
|
||||
// cannot be required outside a browser.
|
||||
|
||||
const { RESTORABLE_VIEWS } = require("./restorableViews");
|
||||
|
||||
// Views that render an address the user picked and cannot be rendered
|
||||
// without one.
|
||||
const ADDRESS_VIEWS = new Set([
|
||||
"address",
|
||||
"address-token",
|
||||
"receive",
|
||||
"transaction",
|
||||
]);
|
||||
|
||||
function needsAddress(view) {
|
||||
return ADDRESS_VIEWS.has(view);
|
||||
}
|
||||
|
||||
function hasValidAddress(state) {
|
||||
return Boolean(
|
||||
state.selectedWallet !== null &&
|
||||
state.selectedAddress !== null &&
|
||||
state.wallets[state.selectedWallet] &&
|
||||
state.wallets[state.selectedWallet].addresses[state.selectedAddress],
|
||||
);
|
||||
}
|
||||
|
||||
// Render `view` from persisted state. Each view module shows itself, so a
|
||||
// true return means the view is both rendered and on screen.
|
||||
//
|
||||
// Returns false when the view is not one the popup renders from state, or
|
||||
// when the state it would render is gone — a token no longer selected, a
|
||||
// transaction no longer persisted. The caller falls back rather than
|
||||
// putting an empty template on screen.
|
||||
function renderView(view, state, views) {
|
||||
if (!view || !RESTORABLE_VIEWS.has(view)) return false;
|
||||
if (needsAddress(view) && !hasValidAddress(state)) return false;
|
||||
if (view === "address-token" && !state.selectedToken) return false;
|
||||
|
||||
const data = state.viewData || {};
|
||||
switch (view) {
|
||||
case "main":
|
||||
views.main.show();
|
||||
return true;
|
||||
case "address":
|
||||
views.addressDetail.show();
|
||||
return true;
|
||||
case "address-token":
|
||||
views.addressToken.show();
|
||||
return true;
|
||||
case "receive":
|
||||
views.receive.show();
|
||||
return true;
|
||||
case "settings":
|
||||
views.settings.show();
|
||||
return true;
|
||||
case "settings-addtoken":
|
||||
views.settingsAddToken.show();
|
||||
return true;
|
||||
case "confirm-tx":
|
||||
if (!data.pendingTx) return false;
|
||||
views.confirmTx.restore();
|
||||
return true;
|
||||
case "transaction":
|
||||
if (!data.tx) return false;
|
||||
views.transactionDetail.render();
|
||||
return true;
|
||||
case "wait-tx":
|
||||
// Resumes the receipt poll from the persisted broadcast time,
|
||||
// and answers false when there is nothing resumable left.
|
||||
return Boolean(views.txStatus.restoreWait());
|
||||
case "success-tx":
|
||||
if (!data.hash) return false;
|
||||
views.txStatus.renderSuccess();
|
||||
return true;
|
||||
case "error-tx":
|
||||
if (!data.message) return false;
|
||||
views.txStatus.renderError();
|
||||
return true;
|
||||
default:
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
// The Back-path renderer, registered with setBackRenderer() in
|
||||
// views/helpers.js. Returns false for a view the popup does not render from
|
||||
// persisted state, leaving goBack() to unhide it: the restored stack is
|
||||
// filtered against RESTORABLE_VIEWS, so such a view can only be on the
|
||||
// stack from this page load, where forward navigation already rendered it.
|
||||
function makeBackRenderer(state, views) {
|
||||
return function renderBack(view) {
|
||||
if (!RESTORABLE_VIEWS.has(view)) return false;
|
||||
if (!renderView(view, state, views)) {
|
||||
views.main.show();
|
||||
}
|
||||
return true;
|
||||
};
|
||||
}
|
||||
|
||||
module.exports = {
|
||||
renderView,
|
||||
makeBackRenderer,
|
||||
needsAddress,
|
||||
hasValidAddress,
|
||||
};
|
||||
@@ -111,12 +111,19 @@ function updateDebugBanner(viewName) {
|
||||
}
|
||||
}
|
||||
|
||||
// Callback to re-render the main/home view when navigating back to it.
|
||||
// Set once by index.js via setRenderMain().
|
||||
let _renderMain = null;
|
||||
// Callback that renders a view being navigated BACK onto. Set once by
|
||||
// index.js via setBackRenderer(), which routes the view through the same
|
||||
// per-view render and data guards restoreView() uses.
|
||||
//
|
||||
// It answers true when it took the navigation — the view is rendered and
|
||||
// shown, or its backing data was gone and it fell back — and false for a
|
||||
// view the popup does not render from persisted state. Those can only be
|
||||
// on the stack from this page load, because the stack is filtered on load,
|
||||
// so they have already been rendered and only need unhiding.
|
||||
let _renderBack = null;
|
||||
|
||||
function setRenderMain(fn) {
|
||||
_renderMain = fn;
|
||||
function setBackRenderer(fn) {
|
||||
_renderBack = fn;
|
||||
}
|
||||
|
||||
// Push the current view onto the navigation stack so goBack() can
|
||||
@@ -136,9 +143,11 @@ function goBack() {
|
||||
} else {
|
||||
target = "main";
|
||||
}
|
||||
if (target === "main" && _renderMain) {
|
||||
_renderMain();
|
||||
}
|
||||
// A popped view is landed on, not navigated to. If the popup has been
|
||||
// closed and reopened since the view was pushed, nothing has ever
|
||||
// rendered it in this page load and its template is still blank, so it
|
||||
// has to be rendered here rather than merely unhidden.
|
||||
if (_renderBack && _renderBack(target)) return;
|
||||
showView(target);
|
||||
}
|
||||
|
||||
@@ -470,7 +479,7 @@ module.exports = {
|
||||
showView,
|
||||
onViewLeave,
|
||||
updateDebugBanner,
|
||||
setRenderMain,
|
||||
setBackRenderer,
|
||||
pushCurrentView,
|
||||
goBack,
|
||||
clearViewStack,
|
||||
|
||||
Reference in New Issue
Block a user