fix(ui): no-op when re-selecting the turn already on screen
Clicking the current-turn row in the header turn navigator while already viewing it routed through returnToCurrent() → viewTurn(currentTurn), which re-fetches the live report and flips the view through `loading`. At turn 0 the only row is the live turn, so the dropdown always fired a pointless backend round-trip and redraw. Guard goToTurn() against re-selecting the on-screen turn (turn === viewedTurn): just close the popover and stop. Leaving history is unaffected — there the viewed turn differs from the target. Closes #45 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -179,8 +179,12 @@ arrows step `viewedTurn` by ±1 (disabled at boundaries `0` and
|
||||
popover (desktop) or a fixed full-width drawer (mobile, ≤ 767.98
|
||||
px) listing every turn from `currentTurn` down to `0`. Selecting
|
||||
the current-turn row routes through `gameState.returnToCurrent()`;
|
||||
any other row calls `gameState.viewTurn(N)`. The popover reuses
|
||||
`view-menu.svelte`'s outside-click / Escape pattern.
|
||||
any other row calls `gameState.viewTurn(N)`. Selecting the row
|
||||
already on screen (`viewedTurn`) is a pure no-op — it only closes
|
||||
the popover — so re-picking the live turn (most visibly turn 0,
|
||||
where it is the only row) never re-fetches the report to redraw the
|
||||
same snapshot. The popover reuses `view-menu.svelte`'s
|
||||
outside-click / Escape pattern.
|
||||
|
||||
`lib/header/history-banner.svelte` renders directly under the
|
||||
header whenever `gameState.historyMode === true`. It shows
|
||||
|
||||
@@ -12,7 +12,10 @@ drawer below the 768 px breakpoint, mirroring `view-menu.svelte`.
|
||||
|
||||
Selecting a row calls `gameState.viewTurn(N)`; the row that matches
|
||||
`currentTurn` delegates to `gameState.returnToCurrent()` so the
|
||||
"leave history" path always flows through one method.
|
||||
"leave history" path always flows through one method. Selecting the
|
||||
row already on screen (`viewedTurn`) is a pure no-op — most visibly
|
||||
at turn 0, where the sole row is the live turn — so the navigator
|
||||
never re-fetches the report just to redraw the same snapshot.
|
||||
-->
|
||||
<script lang="ts">
|
||||
import { getContext, onMount } from "svelte";
|
||||
@@ -64,6 +67,13 @@ Selecting a row calls `gameState.viewTurn(N)`; the row that matches
|
||||
async function goToTurn(turn: number): Promise<void> {
|
||||
open = false;
|
||||
if (gameState === undefined) return;
|
||||
// Re-selecting the turn already on screen changes nothing, so just
|
||||
// close the popover. Without this guard the current-turn row routes
|
||||
// through `returnToCurrent()` → `viewTurn(currentTurn)`, which
|
||||
// re-fetches the live report and flips the view through `loading` —
|
||||
// most visibly at turn 0, where the only row is the live turn.
|
||||
// Leaving history still works: there the viewed turn differs.
|
||||
if (turn === gameState.viewedTurn) return;
|
||||
if (turn === gameState.currentTurn) {
|
||||
await gameState.returnToCurrent();
|
||||
return;
|
||||
|
||||
@@ -165,4 +165,40 @@ describe("TurnNavigator", () => {
|
||||
expect(returnToCurrent).toHaveBeenCalledTimes(1);
|
||||
expect(viewTurn).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test("selecting the live row at turn 0 is a no-op (no fetch, no redraw)", async () => {
|
||||
const store = buildStore({ currentTurn: 0, viewedTurn: 0 });
|
||||
const viewTurn = vi
|
||||
.spyOn(store, "viewTurn")
|
||||
.mockResolvedValue(undefined);
|
||||
const returnToCurrent = vi
|
||||
.spyOn(store, "returnToCurrent")
|
||||
.mockResolvedValue(undefined);
|
||||
const ui = render(TurnNavigator, {
|
||||
context: new Map([[GAME_STATE_CONTEXT_KEY, store]]),
|
||||
});
|
||||
await fireEvent.click(ui.getByTestId("turn-navigator-trigger"));
|
||||
await fireEvent.click(ui.getByTestId("turn-navigator-item-0"));
|
||||
expect(viewTurn).not.toHaveBeenCalled();
|
||||
expect(returnToCurrent).not.toHaveBeenCalled();
|
||||
// The click still dismisses the popover.
|
||||
expect(ui.queryByTestId("turn-navigator-list")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
test("re-selecting the row already viewed in history is a no-op", async () => {
|
||||
const store = buildStore({ currentTurn: 3, viewedTurn: 2 });
|
||||
const viewTurn = vi
|
||||
.spyOn(store, "viewTurn")
|
||||
.mockResolvedValue(undefined);
|
||||
const returnToCurrent = vi
|
||||
.spyOn(store, "returnToCurrent")
|
||||
.mockResolvedValue(undefined);
|
||||
const ui = render(TurnNavigator, {
|
||||
context: new Map([[GAME_STATE_CONTEXT_KEY, store]]),
|
||||
});
|
||||
await fireEvent.click(ui.getByTestId("turn-navigator-trigger"));
|
||||
await fireEvent.click(ui.getByTestId("turn-navigator-item-2"));
|
||||
expect(viewTurn).not.toHaveBeenCalled();
|
||||
expect(returnToCurrent).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user