fix(ui): the Android back button had nowhere to go
Reported from the first device run: back does not navigate back in the app. The scaffold's `MainActivity.onBackPressed` asks `webView.canGoBack()` and finishes the activity otherwise -- and this app had never touched `history`, so that was false at every depth and back quit from anywhere. The fix is here rather than in Java, because the mechanism the scaffold already uses is the one we were failing to feed: a navigation is a history entry now, and `popstate` replays it. Nothing on the Android side changes, and the behaviour becomes assertable in a browser with `page.goBack()` instead of only on a phone. The entry keeps the same URL -- the app has no routes, and a path a reload cannot resolve is worse than none -- and carries the destination in its state. Two rules keep the stacks from disagreeing. The first navigation *replaces* the launch entry rather than pushing one, or every launch costs a back press before the app will close. And the in-app back buttons go through `history.back()` rather than popping a stack of their own: `navStack` is deleted, not kept alongside, because two stacks is precisely how a detail view's own button and the phone's gesture come to disagree about how far one press goes. The third spec pins that invariant.
This commit is contained in:
@@ -0,0 +1,92 @@
|
||||
import { test, expect } from '../support/fixtures.js';
|
||||
|
||||
/**
|
||||
* Back is the platform's, and the app has to have somewhere for it to
|
||||
* go (reported from a device: "the Android back button does not
|
||||
* navigate back in the app").
|
||||
*
|
||||
* The scaffold's `MainActivity.onBackPressed` asks `webView.canGoBack()`
|
||||
* and finishes the activity otherwise. This app never touched
|
||||
* `history`, so that was always false and back quit from any depth. A
|
||||
* navigation is a history entry now, which is why this is assertable
|
||||
* here at all: `page.goBack()` is the same `popstate` the phone's
|
||||
* gesture produces, so the browser tier can answer a question that
|
||||
* otherwise needs a device.
|
||||
*
|
||||
* What it cannot answer is whether Android's *gesture* reaches the
|
||||
* WebView, which is between the OS and the scaffold.
|
||||
*/
|
||||
type Page = import('@playwright/test').Page;
|
||||
|
||||
const activeView = (page: Page) =>
|
||||
page.getByTestId('main-content');
|
||||
|
||||
/**
|
||||
* Open an artist's detail view, which is the deepest ordinary route.
|
||||
*
|
||||
* A library artist opens `explore-artist-details` -- the catalog panel
|
||||
* standing in for a library one, as `explore-link.ts` describes -- and
|
||||
* the view name follows the component, not the source of the click.
|
||||
*/
|
||||
async function openAnArtist(app: Page): Promise<void> {
|
||||
await app.getByTestId('nav-artists').click();
|
||||
await expect(activeView(app)).toHaveAttribute('data-active-view', 'artists');
|
||||
|
||||
// A card, by the name on it: the grid is virtualized and positioned
|
||||
// by transform, so a click at coordinates is a click at whatever
|
||||
// happens to be there.
|
||||
await app.locator('artists-view').getByText('Aurora Fields').first().click();
|
||||
await expect(activeView(app)).toHaveAttribute(
|
||||
'data-active-view',
|
||||
'explore-artist-details',
|
||||
);
|
||||
}
|
||||
|
||||
test.describe('the back gesture', () => {
|
||||
test('leaves a detail view for the view it was opened from', async ({
|
||||
app,
|
||||
}) => {
|
||||
await openAnArtist(app);
|
||||
|
||||
await app.goBack();
|
||||
|
||||
await expect(activeView(app)).toHaveAttribute('data-active-view', 'artists');
|
||||
});
|
||||
|
||||
test('walks back through primary views, one press per navigation', async ({
|
||||
app,
|
||||
}) => {
|
||||
await app.getByTestId('nav-tracks').click();
|
||||
await expect(activeView(app)).toHaveAttribute('data-active-view', 'tracks');
|
||||
|
||||
await app.getByTestId('nav-albums').click();
|
||||
await expect(activeView(app)).toHaveAttribute('data-active-view', 'albums');
|
||||
|
||||
await app.goBack();
|
||||
await expect(activeView(app)).toHaveAttribute('data-active-view', 'tracks');
|
||||
|
||||
// Forward is free once back works, and it is what proves the entry
|
||||
// was restored rather than the view merely re-rendered.
|
||||
await app.goForward();
|
||||
await expect(activeView(app)).toHaveAttribute('data-active-view', 'albums');
|
||||
});
|
||||
|
||||
test('an in-app back button consumes exactly one entry', async ({ app }) => {
|
||||
await app.getByTestId('nav-tracks').click();
|
||||
await openAnArtist(app);
|
||||
|
||||
// The detail view's own back button and the phone's gesture are the
|
||||
// same press: if each popped its own stack, this would land two
|
||||
// navigations back instead of one.
|
||||
await app
|
||||
.locator('explore-artist-details')
|
||||
.getByRole('button', { name: 'Back to explore' })
|
||||
.click();
|
||||
|
||||
await expect(activeView(app)).toHaveAttribute('data-active-view', 'artists');
|
||||
|
||||
await app.goBack();
|
||||
|
||||
await expect(activeView(app)).toHaveAttribute('data-active-view', 'tracks');
|
||||
});
|
||||
});
|
||||
+76
-25
@@ -160,10 +160,6 @@ const viewCache = new Map<string, HTMLElement>();
|
||||
let currentViewEl: HTMLElement | null = null;
|
||||
let currentDetailEl: HTMLElement | null = null;
|
||||
|
||||
/** Navigation history stack for back-button support in detail views. */
|
||||
const navStack: Array<{ view: string; [key: string]: any }> = [];
|
||||
/** The current navigation detail (so we can push it onto the stack). */
|
||||
let currentNavDetail: { view: string; [key: string]: any } = { view: 'home' };
|
||||
|
||||
const mainContent = document.getElementById('main-content');
|
||||
|
||||
@@ -196,6 +192,71 @@ document.addEventListener('navigate', (e: Event) => {
|
||||
void handleNavigate((e as CustomEvent).detail);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// The platform's back gesture
|
||||
// ---------------------------------------------------------------------------
|
||||
// Android's back button is not a keystroke the page can bind: the
|
||||
// scaffold's `MainActivity.onBackPressed` asks `webView.canGoBack()` and
|
||||
// otherwise finishes the activity. This app never touched `history`, so
|
||||
// that was always false and back quit the app from any depth -- reported
|
||||
// from a device as "back does not navigate back".
|
||||
//
|
||||
// So a navigation is a history entry, and back is `popstate`. It hooks
|
||||
// the platform's own mechanism rather than a JNI callback of our own,
|
||||
// which is the same reason `events.ts` hooks the runtime's transport:
|
||||
// the Java half needs no change, and the behaviour is testable in a
|
||||
// browser (`page.goBack()`) instead of only on a phone.
|
||||
//
|
||||
// Two rules keep the two stacks from disagreeing. A navigation that
|
||||
// *came from* history pushes nothing (`_isBack`), or going back would
|
||||
// deepen the stack it is unwinding. And the in-app back buttons --
|
||||
// `navigate-back`, which the detail views and `now-playing-view` fire --
|
||||
// go through `history.back()` rather than popping `navStack`
|
||||
// themselves, so one press cannot consume two entries.
|
||||
|
||||
/** The navigation an entry stands for. `undefined` on the entry that
|
||||
* predates the app's own routing, which is the one back exits from. */
|
||||
type NavState = { yjNav?: { view: string; [key: string]: any } };
|
||||
|
||||
/** Whether the app's first navigation has been recorded. It *replaces*
|
||||
* the launch entry rather than pushing, or every launch would cost one
|
||||
* back press before the app would exit. */
|
||||
let historyStarted = false;
|
||||
|
||||
/** How many entries this session has pushed beyond that first one --
|
||||
* i.e. how deep back can go while staying inside the app. */
|
||||
let pushedEntries = 0;
|
||||
|
||||
function recordNavigation(detail: { view: string; [key: string]: any }): void {
|
||||
// `_isBack` is bookkeeping, not destination: keeping it in the entry
|
||||
// would make a replayed navigation claim to be a back-navigation.
|
||||
const { _isBack: _ignored, ...nav } = detail;
|
||||
const state: NavState = { yjNav: nav };
|
||||
|
||||
// Same URL, deliberately: the app has no routes, and a path a
|
||||
// reload cannot resolve is worse than no path at all.
|
||||
if (historyStarted) {
|
||||
history.pushState(state, '');
|
||||
pushedEntries += 1;
|
||||
} else {
|
||||
history.replaceState(state, '');
|
||||
historyStarted = true;
|
||||
}
|
||||
}
|
||||
|
||||
window.addEventListener('popstate', (e: PopStateEvent) => {
|
||||
const nav = (e.state as NavState | null)?.yjNav;
|
||||
|
||||
// Before the app's first navigation, or an entry somebody else
|
||||
// pushed: nothing to restore, and the activity should be free to
|
||||
// finish.
|
||||
if (!nav) return;
|
||||
|
||||
pushedEntries = Math.max(0, pushedEntries - 1);
|
||||
|
||||
void handleNavigate({ ...nav, _isBack: true });
|
||||
});
|
||||
|
||||
async function handleNavigate(
|
||||
detail: { view: string; [key: string]: any },
|
||||
): Promise<void> {
|
||||
@@ -205,6 +266,8 @@ async function handleNavigate(
|
||||
|
||||
const seq = ++navSeq;
|
||||
|
||||
if (!detail._isBack) recordNavigation(detail);
|
||||
|
||||
// Bookkeeping stays synchronous with the click: the search box's
|
||||
// scope and the active-view attribute describe the navigation that
|
||||
// was *asked for*, and are what the rest of the app and the e2e
|
||||
@@ -218,9 +281,6 @@ async function handleNavigate(
|
||||
|
||||
// --- Primary (cacheable) views ----------------------------------------
|
||||
if (view in VIEW_TAGS) {
|
||||
// Navigating to a primary view clears the history stack.
|
||||
navStack.length = 0;
|
||||
|
||||
// Remove any active detail view first
|
||||
if (currentDetailEl) {
|
||||
deactivateView(currentDetailEl);
|
||||
@@ -253,7 +313,6 @@ async function handleNavigate(
|
||||
// the way out. Either way this is the call that starts it.
|
||||
activateView(target);
|
||||
currentViewEl = target;
|
||||
currentNavDetail = { view };
|
||||
|
||||
return;
|
||||
}
|
||||
@@ -262,12 +321,6 @@ async function handleNavigate(
|
||||
if (seq !== navSeq) return;
|
||||
|
||||
// --- Detail (ephemeral) views -----------------------------------------
|
||||
// Push the current view onto the nav stack before switching
|
||||
// (unless this is a back-navigation, which already popped).
|
||||
if (!detail._isBack) {
|
||||
navStack.push({ ...currentNavDetail });
|
||||
}
|
||||
|
||||
// Hide the current primary view
|
||||
if (currentViewEl) {
|
||||
currentViewEl.classList.add('view-hidden');
|
||||
@@ -280,8 +333,6 @@ async function handleNavigate(
|
||||
currentDetailEl = null;
|
||||
}
|
||||
|
||||
currentNavDetail = { ...detail };
|
||||
|
||||
switch (view) {
|
||||
case 'artist-details': {
|
||||
const { artistId, artistName } = detail;
|
||||
@@ -425,16 +476,16 @@ function schedule(fn: () => void): void {
|
||||
setTimeout(fn, 200);
|
||||
}
|
||||
|
||||
// Navigate-back: pop the nav stack and re-dispatch as a regular navigate.
|
||||
// Navigate-back: the in-app back buttons, which are the same press as
|
||||
// the phone's. It goes through the history rather than a stack of its
|
||||
// own, so one press is one entry however it arrived -- two stacks is
|
||||
// how a detail view's own button and the back gesture come to disagree.
|
||||
//
|
||||
// At the root there is nothing of ours to go back to, and going back
|
||||
// anyway would leave the app: the depth check is what stops a stray
|
||||
// `navigate-back` closing it.
|
||||
document.addEventListener('navigate-back', () => {
|
||||
const prev = navStack.pop();
|
||||
if (prev) {
|
||||
document.dispatchEvent(new CustomEvent('navigate', {
|
||||
bubbles: true,
|
||||
composed: true,
|
||||
detail: { ...prev, _isBack: true },
|
||||
}));
|
||||
}
|
||||
if (pushedEntries > 0) history.back();
|
||||
});
|
||||
|
||||
// Navigate to the user's configured launch page. Falls back to 'home'
|
||||
|
||||
Reference in New Issue
Block a user