[fix](trx-frontend-http): stop Tools lighting up on every refresh
navigateToTab marked the Tools button by asking whether the destination tab was displayed, which is the right question at the wrong moment: the first route navigation runs while the card is still behind the loading state, where every tab computes to display:none. Refreshing or deep linking to any page therefore lit Tools alongside the real destination, and nothing re-evaluated it once the page appeared. Membership of the Tools menu answers the same question without needing anything laid out, and still reads the grouping ui-core installs rather than a second copy of it. The smoke fixture now serves the SPA shell for route paths the way the server's per-tab index handlers do, so a deep link no longer 404s and the case is testable at all; two of them are asserted. Signed-off-by: Stan Grams <sjg@haxx.space>
This commit is contained in:
@@ -5604,7 +5604,10 @@ function navigateToTab(name, options = {}) {
|
|||||||
document.querySelectorAll(".tab-bar .tab").forEach((t) => t.classList.remove("active"));
|
document.querySelectorAll(".tab-bar .tab").forEach((t) => t.classList.remove("active"));
|
||||||
btn.classList.add("active");
|
btn.classList.add("active");
|
||||||
const toolsBtn = document.getElementById("mobile-more-btn");
|
const toolsBtn = document.getElementById("mobile-more-btn");
|
||||||
if (toolsBtn) toolsBtn.classList.toggle("active", getComputedStyle(btn).display === "none");
|
if (toolsBtn) {
|
||||||
|
const inToolsMenu = !!document.querySelector(`#mobile-more-menu [data-navigate-tab="${name}"]`);
|
||||||
|
toolsBtn.classList.toggle("active", inToolsMenu);
|
||||||
|
}
|
||||||
window.trxUi?.syncSelectedTab(document.querySelector(".tab-bar-nav"), btn);
|
window.trxUi?.syncSelectedTab(document.querySelector(".tab-bar-nav"), btn);
|
||||||
document.querySelectorAll(".tab-panel").forEach((p) => p.style.display = "none");
|
document.querySelectorAll(".tab-panel").forEach((p) => p.style.display = "none");
|
||||||
const panel = document.getElementById(`tab-${name}`);
|
const panel = document.getElementById(`tab-${name}`);
|
||||||
|
|||||||
@@ -4618,10 +4618,17 @@ function navigateToTab(name: TabName, options: { updateHistory?: boolean; replac
|
|||||||
btn.classList.add("active");
|
btn.classList.add("active");
|
||||||
// A destination the strip hides is reached through Tools, so mark that
|
// A destination the strip hides is reached through Tools, so mark that
|
||||||
// button instead — otherwise the strip looks identical on all four of them.
|
// button instead — otherwise the strip looks identical on all four of them.
|
||||||
// Derived from what is actually hidden rather than from a second copy of the
|
// Membership of the Tools menu is the test: it still reads from the grouping
|
||||||
// grouping, which would drift from the one ui-core installs.
|
// ui-core installs rather than a second copy of it, but unlike the tab's
|
||||||
|
// computed display it does not depend on anything being laid out. The first
|
||||||
|
// route navigation runs while the card is still behind the loading state,
|
||||||
|
// where every tab computes to display:none — which lit Tools up on every
|
||||||
|
// refresh of every page.
|
||||||
const toolsBtn = document.getElementById("mobile-more-btn");
|
const toolsBtn = document.getElementById("mobile-more-btn");
|
||||||
if (toolsBtn) toolsBtn.classList.toggle("active", getComputedStyle(btn).display === "none");
|
if (toolsBtn) {
|
||||||
|
const inToolsMenu = !!document.querySelector(`#mobile-more-menu [data-navigate-tab="${name}"]`);
|
||||||
|
toolsBtn.classList.toggle("active", inToolsMenu);
|
||||||
|
}
|
||||||
window.trxUi?.syncSelectedTab(document.querySelector(".tab-bar-nav"), btn);
|
window.trxUi?.syncSelectedTab(document.querySelector(".tab-bar-nav"), btn);
|
||||||
document.querySelectorAll<HTMLElement>(".tab-panel").forEach((p) => p.style.display = "none");
|
document.querySelectorAll<HTMLElement>(".tab-panel").forEach((p) => p.style.display = "none");
|
||||||
const panel = document.getElementById(`tab-${name}`);
|
const panel = document.getElementById(`tab-${name}`);
|
||||||
|
|||||||
@@ -124,7 +124,9 @@ const contentTypes = new Map([
|
|||||||
]);
|
]);
|
||||||
|
|
||||||
function assetPath(urlPath) {
|
function assetPath(urlPath) {
|
||||||
if (urlPath === "/") return path.join(webDir, "index.html");
|
// Every tab route has its own index handler on the server (see api/assets.rs),
|
||||||
|
// so a deep link or a refresh serves the SPA shell, not a 404.
|
||||||
|
if (!path.extname(urlPath)) return path.join(webDir, "index.html");
|
||||||
if (urlPath.startsWith("/vendor/")) return path.join(webDir, urlPath);
|
if (urlPath.startsWith("/vendor/")) return path.join(webDir, urlPath);
|
||||||
const generated = path.join(generatedDir, path.basename(urlPath));
|
const generated = path.join(generatedDir, path.basename(urlPath));
|
||||||
if (urlPath.endsWith(".js")) return generated;
|
if (urlPath.endsWith(".js")) return generated;
|
||||||
@@ -265,6 +267,24 @@ try {
|
|||||||
assert.equal(new URL(page.url()).pathname, "/");
|
assert.equal(new URL(page.url()).pathname, "/");
|
||||||
assert.deepEqual(runtimeErrors, []);
|
assert.deepEqual(runtimeErrors, []);
|
||||||
|
|
||||||
|
// Refreshing or deep-linking must mark the destination, not Tools. The first
|
||||||
|
// route navigation runs while the card is still behind the loading state, so
|
||||||
|
// a test that asked whether the tab was displayed saw "none" for every tab
|
||||||
|
// and lit Tools up on every refresh of every page.
|
||||||
|
for (const [route, tab, toolsLit] of [["/map", "map", false], ["/about", "about", true]]) {
|
||||||
|
await page.goto(`http://127.0.0.1:${address.port}${route}`, { waitUntil: "domcontentloaded" });
|
||||||
|
await page.locator(`#tab-${tab}`).waitFor({ state: "visible" });
|
||||||
|
const marked = await page.evaluate(() => ({
|
||||||
|
actives: [...document.querySelectorAll(".tab-bar .tab.active")].map((t) => t.dataset.tab || t.id),
|
||||||
|
tools: document.getElementById("mobile-more-btn").classList.contains("active"),
|
||||||
|
}));
|
||||||
|
assert.ok(marked.actives.includes(tab), `${route} marks ${JSON.stringify(marked.actives)}`);
|
||||||
|
assert.equal(marked.tools, toolsLit, `${route}: Tools active is ${marked.tools}`);
|
||||||
|
}
|
||||||
|
await page.goto(`http://127.0.0.1:${address.port}/`, { waitUntil: "domcontentloaded" });
|
||||||
|
await page.locator("#tab-main").waitFor({ state: "visible" });
|
||||||
|
assert.deepEqual(runtimeErrors, []);
|
||||||
|
|
||||||
// --- Layout regressions -------------------------------------------------
|
// --- Layout regressions -------------------------------------------------
|
||||||
// Every fault below shipped at some point while the rest of this file
|
// Every fault below shipped at some point while the rest of this file
|
||||||
// passed, because nothing here looked at geometry: a header whose height
|
// passed, because nothing here looked at geometry: a header whose height
|
||||||
|
|||||||
Reference in New Issue
Block a user