fix(nav): an item Back finds in another list is brought into view
This commit is contained in:
@@ -5,7 +5,7 @@
|
|||||||
// sits in the stream below. The restore matched the unnamed list by its place, missed the name there
|
// sits in the stream below. The restore matched the unnamed list by its place, missed the name there
|
||||||
// and fell back to the saved index — the focus landed on the neighbour, and Enter opened the wrong
|
// and fell back to the saved index — the focus landed on the neighbour, and Enter opened the wrong
|
||||||
// notification. Its own file: listNav keeps module-level state.
|
// notification. Its own file: listNav keeps module-level state.
|
||||||
import { afterEach, describe, expect, it } from "vitest";
|
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||||
import { act, cleanup, render } from "@testing-library/react";
|
import { act, cleanup, render } from "@testing-library/react";
|
||||||
import { useRef } from "react";
|
import { useRef } from "react";
|
||||||
|
|
||||||
@@ -72,4 +72,21 @@ describe("Back to an item that moved to another list", () => {
|
|||||||
]);
|
]);
|
||||||
expect(focused()).toBe("download");
|
expect(focused()).toBe("download");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("brings an item found in another list into view; one in its own list stays put", async () => {
|
||||||
|
// The scroll restore put the page where the item WAS: found elsewhere, it would hold the focus
|
||||||
|
// off screen.
|
||||||
|
const reveal = vi.fn();
|
||||||
|
Element.prototype.scrollIntoView = reveal;
|
||||||
|
await leaveAndReturn([["subscribed", "download"], ["older"]], "subscribed", [
|
||||||
|
["download"],
|
||||||
|
["subscribed", "older"],
|
||||||
|
]);
|
||||||
|
expect(reveal).toHaveBeenCalledTimes(1);
|
||||||
|
cleanup();
|
||||||
|
reveal.mockClear();
|
||||||
|
await leaveAndReturn([["subscribed", "download"]], "download", [["download", "subscribed"]]);
|
||||||
|
expect(focused()).toBe("download");
|
||||||
|
expect(reveal).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -492,7 +492,8 @@ function restoreTarget(
|
|||||||
* list N may be another list, and <body> is the honest outcome — unless the item's name is the
|
* list N may be another list, and <body> is the honest outcome — unless the item's name is the
|
||||||
* page's alone (`restoreTarget`). Gives up when the time is up, and at
|
* page's alone (`restoreTarget`). Gives up when the time is up, and at
|
||||||
* once when focus is already somewhere (the reader moved on, a dialog took it). The item is focused
|
* once when focus is already somewhere (the reader moved on, a dialog took it). The item is focused
|
||||||
* where the scroll restore put it, never scrolled to. (A stand-in the focus keeper placed when the view
|
* where the scroll restore put it, never scrolled to — but one found in another list is brought into
|
||||||
|
* view. (A stand-in the focus keeper placed when the view
|
||||||
* it held was swapped away counts as untouched, like <body>.) Returns a cancel. */
|
* it held was swapped away counts as untouched, like <body>.) Returns a cancel. */
|
||||||
export function restoreNavPosition(pos: NavPosition): () => void {
|
export function restoreNavPosition(pos: NavPosition): () => void {
|
||||||
const deadline = Date.now() + RESTORE_MS;
|
const deadline = Date.now() + RESTORE_MS;
|
||||||
@@ -512,7 +513,9 @@ export function restoreNavPosition(pos: NavPosition): () => void {
|
|||||||
// nearest one: names repeat (a log console's `http.request GET /api/feed 200` lines, two
|
// nearest one: names repeat (a log console's `http.request GET /api/feed 200` lines, two
|
||||||
// downloads of one video), and the first of them is rarely the one left behind.
|
// downloads of one video), and the first of them is rarely the one left behind.
|
||||||
const el = restoreTarget(all, list, pos);
|
const el = restoreTarget(all, list, pos);
|
||||||
if (el) return focusItem(el, false);
|
// Where the scroll restore put it — unless it is found in another list than the one it was left
|
||||||
|
// in: the restored scroll is the old place's, and the item would hold the focus off screen.
|
||||||
|
if (el) return focusItem(el, !list?.el.contains(el));
|
||||||
if (Date.now() < deadline) timer = setTimeout(attempt, RESTORE_POLL_MS);
|
if (Date.now() < deadline) timer = setTimeout(attempt, RESTORE_POLL_MS);
|
||||||
};
|
};
|
||||||
// Not inside the popstate dispatch that landed: a sub-view's own listener (`useHistorySubview`)
|
// Not inside the popstate dispatch that landed: a sub-view's own listener (`useHistorySubview`)
|
||||||
|
|||||||
Reference in New Issue
Block a user