fix: Back from Settings never lands on Settings itself (closes #481)
The recovery phrase and delete wallet screens take themselves off the Back stack when left, so Settings, one of those screens, then the settings gear leaves Settings under the Settings now showing. A reopened popup restores the same stack, cut at the screen the gear left. Back then showed Settings again and seemed to do nothing. goBack() now skips any entry for the screen already showing before it pops its target. Jest tests drive the gear and then Back for the recovery phrase screen, once and twice over, for both delete screens, and after a reopen. Model: opus-5-5
This commit is contained in:
@@ -1283,11 +1283,14 @@ behind a "···" menu.
|
|||||||
|
|
||||||
Navigation uses a stack model (like iOS): each forward action pushes the current
|
Navigation uses a stack model (like iOS): each forward action pushes the current
|
||||||
screen onto `state.viewStack`, and "Back" pops it (`pushCurrentView()` and
|
screen onto `state.viewStack`, and "Back" pops it (`pushCurrentView()` and
|
||||||
`goBack()` in `src/popup/views/helpers.js`). The root screen is either Welcome
|
`goBack()` in `src/popup/views/helpers.js`). "Back" skips an entry for the
|
||||||
(no wallets) or Home (has wallets). Each screen below gives its view id in
|
screen already showing: ShowRecoveryPhrase and the two delete screens take
|
||||||
parentheses; the registry of view ids is the `VIEWS` array in
|
themselves off the stack when left, so the Settings gear on one of them leaves
|
||||||
`src/popup/views/helpers.js`, and the markup for a screen is the element with id
|
Settings under Settings, and "Back" from there goes to the screen before
|
||||||
`view-` plus that view id in `src/popup/index.html`.
|
Settings. The root screen is either Welcome (no wallets) or Home (has wallets).
|
||||||
|
Each screen below gives its view id in parentheses; the registry of view ids is
|
||||||
|
the `VIEWS` array in `src/popup/views/helpers.js`, and the markup for a screen
|
||||||
|
is the element with id `view-` plus that view id in `src/popup/index.html`.
|
||||||
|
|
||||||
Three elements sit outside the screens and are present on all of them: the title
|
Three elements sit outside the screens and are present on all of them: the title
|
||||||
bar ("AutistMask by @sneak" plus the Settings gear), the flash message line
|
bar ("AutistMask by @sneak" plus the Settings gear), the flash message line
|
||||||
|
|||||||
@@ -45,6 +45,15 @@ but the review is broader than any of them.
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-10-07: Back from Settings no longer shows Settings again after the
|
||||||
|
settings gear was pressed on the recovery phrase or a delete wallet screen
|
||||||
|
opened from Settings, with or without a reopen in between
|
||||||
|
([#481](https://git.eeqj.de/sneak/AutistMask/issues/481)). Those screens take
|
||||||
|
themselves off the Back stack when left, which leaves Settings under Settings;
|
||||||
|
`goBack()` now skips an entry for the screen already showing.
|
||||||
|
`tests/showPhrase.test.js`, `tests/deleteWalletLostPassword.test.js` and
|
||||||
|
`tests/backNavigation.test.js` drive each path.
|
||||||
|
|
||||||
- 2026-10-06: Back from Settings no longer lands on the delete wallet or
|
- 2026-10-06: Back from Settings no longer lands on the delete wallet or
|
||||||
lost-password screen after either was left by the settings gear
|
lost-password screen after either was left by the settings gear
|
||||||
([#480](https://git.eeqj.de/sneak/AutistMask/issues/480)), the defect
|
([#480](https://git.eeqj.de/sneak/AutistMask/issues/480)), the defect
|
||||||
|
|||||||
@@ -216,10 +216,21 @@ function pushCurrentView() {
|
|||||||
|
|
||||||
// Pop the navigation stack and show the previous view. If the stack
|
// Pop the navigation stack and show the previous view. If the stack
|
||||||
// is empty, fall back to the main (home) view.
|
// is empty, fall back to the main (home) view.
|
||||||
|
//
|
||||||
|
// An entry for the view already showing is skipped: landing on it would
|
||||||
|
// make Back seem to do nothing. Settings, the recovery phrase or delete
|
||||||
|
// wallet screen, then the gear leaves Settings under Settings, because that
|
||||||
|
// screen takes itself off the stack when left, and a reopened popup cuts it
|
||||||
|
// off the restored stack the same way
|
||||||
|
// (https://git.eeqj.de/sneak/AutistMask/issues/481).
|
||||||
function goBack() {
|
function goBack() {
|
||||||
|
const stack = state.viewStack;
|
||||||
|
while (stack.length > 0 && stack[stack.length - 1] === state.currentView) {
|
||||||
|
stack.pop();
|
||||||
|
}
|
||||||
let target;
|
let target;
|
||||||
if (state.viewStack.length > 0) {
|
if (stack.length > 0) {
|
||||||
target = state.viewStack.pop();
|
target = stack.pop();
|
||||||
} else {
|
} else {
|
||||||
target = "main";
|
target = "main";
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -52,6 +52,7 @@ const {
|
|||||||
resetRenderedViews,
|
resetRenderedViews,
|
||||||
} = require("../src/popup/viewRouter");
|
} = require("../src/popup/viewRouter");
|
||||||
const { state } = require("../src/shared/state");
|
const { state } = require("../src/shared/state");
|
||||||
|
const { restorableStack } = require("../src/shared/persistedState");
|
||||||
|
|
||||||
const ADDRESS = "0x1111111111111111111111111111111111111111";
|
const ADDRESS = "0x1111111111111111111111111111111111111111";
|
||||||
const TOKEN = "0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48";
|
const TOKEN = "0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48";
|
||||||
@@ -209,6 +210,23 @@ describe("Back onto a view the reopened popup never rendered", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// https://git.eeqj.de/sneak/AutistMask/issues/481. Settings, the recovery
|
||||||
|
// phrase or delete wallet screen, the gear, then a reopen: the restored stack
|
||||||
|
// is cut at the screen the gear left, which leaves Settings under the Settings
|
||||||
|
// the popup reopens onto.
|
||||||
|
describe("Back from Settings reopened over its own entry", () => {
|
||||||
|
test.each(["show-phrase", "delete-wallet-confirm"])(
|
||||||
|
"goes to the screen under it after leaving %s",
|
||||||
|
(left) => {
|
||||||
|
const stored = ["main", "settings", left];
|
||||||
|
reopenedOn("settings", restorableStack(stored, "settings"));
|
||||||
|
goBack();
|
||||||
|
expect(calls).toEqual(["main"]);
|
||||||
|
expect(state.currentView).toBe("main");
|
||||||
|
},
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
// The guards are restoreView()'s, so a popped view whose backing data is
|
// The guards are restoreView()'s, so a popped view whose backing data is
|
||||||
// gone lands on Home rather than on an empty template.
|
// gone lands on Home rather than on an empty template.
|
||||||
describe("Back onto a view whose backing data is gone", () => {
|
describe("Back onto a view whose backing data is gone", () => {
|
||||||
|
|||||||
@@ -618,9 +618,11 @@ describe("the password route's confirm button", () => {
|
|||||||
|
|
||||||
// https://git.eeqj.de/sneak/AutistMask/issues/480: leaving either delete
|
// https://git.eeqj.de/sneak/AutistMask/issues/480: leaving either delete
|
||||||
// screen drops its wallet selection, so Back onto one showed a screen whose
|
// screen drops its wallet selection, so Back onto one showed a screen whose
|
||||||
// button could only answer "No wallet selected for deletion."
|
// button could only answer "No wallet selected for deletion." Taking the
|
||||||
|
// screen off the stack leaves Settings under Settings, and Back must not land
|
||||||
|
// there either (https://git.eeqj.de/sneak/AutistMask/issues/481).
|
||||||
describe("Back from Settings after leaving by the settings gear", () => {
|
describe("Back from Settings after leaving by the settings gear", () => {
|
||||||
test("does not land on the delete screen", () => {
|
test("goes past the delete screen to the screen under Settings", () => {
|
||||||
const { helpers, deleteWallet, state } = load();
|
const { helpers, deleteWallet, state } = load();
|
||||||
deleteWallet.show(1);
|
deleteWallet.show(1);
|
||||||
// The settings gear: push the current view, then show Settings.
|
// The settings gear: push the current view, then show Settings.
|
||||||
@@ -629,10 +631,10 @@ describe("Back from Settings after leaving by the settings gear", () => {
|
|||||||
|
|
||||||
expect(state.viewStack).toEqual(["main", "settings"]);
|
expect(state.viewStack).toEqual(["main", "settings"]);
|
||||||
helpers.goBack();
|
helpers.goBack();
|
||||||
expect(state.currentView).not.toBe("delete-wallet-confirm");
|
expect(state.currentView).toBe("main");
|
||||||
});
|
});
|
||||||
|
|
||||||
test("does not land on the lost-password screen", async () => {
|
test("goes past the lost-password screen to the screen under Settings", async () => {
|
||||||
const { helpers, deleteWallet, state } = load();
|
const { helpers, deleteWallet, state } = load();
|
||||||
await openLostPassword(deleteWallet, 1);
|
await openLostPassword(deleteWallet, 1);
|
||||||
// The settings gear: push the current view, then show Settings.
|
// The settings gear: push the current view, then show Settings.
|
||||||
@@ -641,7 +643,7 @@ describe("Back from Settings after leaving by the settings gear", () => {
|
|||||||
|
|
||||||
expect(state.viewStack).toEqual(["main", "settings"]);
|
expect(state.viewStack).toEqual(["main", "settings"]);
|
||||||
helpers.goBack();
|
helpers.goBack();
|
||||||
expect(state.currentView).not.toBe(VIEW);
|
expect(state.currentView).toBe("main");
|
||||||
});
|
});
|
||||||
|
|
||||||
// The lost-password screen's own Back is "Back returns to the delete
|
// The lost-password screen's own Back is "Back returns to the delete
|
||||||
|
|||||||
@@ -131,8 +131,10 @@ describe("Back from Settings after leaving by the settings gear", () => {
|
|||||||
|
|
||||||
// https://git.eeqj.de/sneak/AutistMask/issues/461: leaving drops the
|
// https://git.eeqj.de/sneak/AutistMask/issues/461: leaving drops the
|
||||||
// wallet selection, so Back onto this screen showed a password prompt
|
// wallet selection, so Back onto this screen showed a password prompt
|
||||||
// that could only answer "No wallet is selected."
|
// that could only answer "No wallet is selected." Taking the screen off
|
||||||
test("does not land on the recovery phrase screen", () => {
|
// the stack leaves Settings under Settings, and Back must not land there
|
||||||
|
// either (https://git.eeqj.de/sneak/AutistMask/issues/481).
|
||||||
|
test("goes to the screen under Settings", () => {
|
||||||
const { helpers, state, showPhrase } = load();
|
const { helpers, state, showPhrase } = load();
|
||||||
|
|
||||||
// Opened from the wallet list in Settings, then left by the gear:
|
// Opened from the wallet list in Settings, then left by the gear:
|
||||||
@@ -143,7 +145,21 @@ describe("Back from Settings after leaving by the settings gear", () => {
|
|||||||
|
|
||||||
expect(state.viewStack).toEqual(["main", "settings"]);
|
expect(state.viewStack).toEqual(["main", "settings"]);
|
||||||
helpers.goBack();
|
helpers.goBack();
|
||||||
expect(state.currentView).not.toBe(SHOW_PHRASE_VIEW);
|
expect(state.currentView).toBe("main");
|
||||||
|
});
|
||||||
|
|
||||||
|
// Each round trip leaves one more Settings under Settings.
|
||||||
|
test("goes to the screen under Settings after two round trips", () => {
|
||||||
|
const { helpers, state, showPhrase } = load();
|
||||||
|
for (let i = 0; i < 2; i++) {
|
||||||
|
showPhrase.show(0);
|
||||||
|
helpers.pushCurrentView();
|
||||||
|
helpers.showView("settings");
|
||||||
|
}
|
||||||
|
|
||||||
|
expect(state.viewStack).toEqual(["main", "settings", "settings"]);
|
||||||
|
helpers.goBack();
|
||||||
|
expect(state.currentView).toBe("main");
|
||||||
});
|
});
|
||||||
|
|
||||||
// This Back takes Settings off the stack before the screen is left, so
|
// This Back takes Settings off the stack before the screen is left, so
|
||||||
|
|||||||
Reference in New Issue
Block a user