FN-6167: fix stale mobile modal navigation history
Keep modal history entries in sync so mobile swipe-back reliably closes reopened task detail views. - add removeNav support to navigation history and ignore self-triggered popstate callbacks - route modal and overlay programmatic close handlers through navigation-aware wrappers - add regression coverage for close-and-reopen mobile detail flows and document the stale stack bug Files changed: .../navigation-history-stale-modal-stack.md | 85 +++++++++++++ packages/dashboard/app/App.tsx | 35 ++++-- packages/dashboard/app/components/AppModals.tsx | 137 +++++++++++++++++---- .../app/components/__tests__/AppModals.test.tsx | 5 +- .../__tests__/navigation-history.test.tsx | 113 ++++++++++++++++- .../hooks/__tests__/useNavigationHistory.test.ts | 98 +++++++++++++++ .../dashboard/app/hooks/useNavigationHistory.ts | 55 ++++++++- 7 files changed, 494 insertions(+), 34 deletions(-) Fusion-Task-Id: FN-6167 Fusion-Task-Lineage: 086d60ba-7e3d-4e83-b9b4-37ec49b06393
This commit is contained in:
@@ -0,0 +1,85 @@
|
||||
---
|
||||
title: "Navigation history stale modal stack"
|
||||
date: 2026-06-09
|
||||
category: ui-bugs
|
||||
module: packages/dashboard/app/hooks/useNavigationHistory
|
||||
problem_type: ui_bug
|
||||
component: frontend_navigation
|
||||
symptoms:
|
||||
- "Mobile browser swipe-back sometimes does not close a reopened task detail modal"
|
||||
- "Browser back works after a fresh modal open but becomes inconsistent after closing with the X button"
|
||||
root_cause: state_desync
|
||||
resolution_type: code_fix
|
||||
severity: medium
|
||||
related_components:
|
||||
- packages/dashboard/app/components/AppModals.tsx
|
||||
- packages/dashboard/app/App.tsx
|
||||
- packages/dashboard/app/hooks/useModalManager.ts
|
||||
tags:
|
||||
- history-api
|
||||
- popstate
|
||||
- modal-navigation
|
||||
- mobile-safari
|
||||
- stale-stack
|
||||
---
|
||||
|
||||
# Navigation history stale modal stack
|
||||
|
||||
## Problem
|
||||
|
||||
`useNavigationHistory` mirrors modal/view opens into `window.history.pushState()` so browser back and mobile swipe-back can close the top modal or revert an overlay view. The internal stack must stay aligned with browser history entries.
|
||||
|
||||
A task detail modal could be opened, closed with the rendered close affordance, opened again, and then fail to close on mobile swipe-back. The close affordance updated modal state but did not remove the corresponding navigation entry, leaving a stale callback on the hook's stack.
|
||||
|
||||
## Symptoms
|
||||
|
||||
- Fresh open → browser back/swipe-back closes the modal.
|
||||
- Open → close with X/mobile back button → reopen → swipe-back can no-op.
|
||||
- The `popstate` handler sees a target `navIndex` that no longer matches the stale internal stack and returns without invoking the close callback.
|
||||
- The issue is most visible on mobile because the fullscreen task detail modal has no overlay tap target; the browser gesture is the primary touch dismiss path.
|
||||
|
||||
## Root cause
|
||||
|
||||
`pushNav({ type: "modal", close })` added both:
|
||||
|
||||
1. an internal `NavEntry` in `useNavigationHistory`, and
|
||||
2. a browser history entry via `history.pushState({ navIndex })`.
|
||||
|
||||
When a modal closed programmatically (X button, mobile header back button, state-driven close), only React/modal state changed. The hook stack retained the old `NavEntry`, so future opens produced duplicate or misindexed stack state. Browser history still moved independently, and `popstate` index arithmetic could silently bail out.
|
||||
|
||||
## Solution
|
||||
|
||||
Add `removeNav(closeOrRevert)` to `useNavigationHistory` and call it from programmatic close paths before the actual close callback:
|
||||
|
||||
```tsx
|
||||
const closeDetailWithNav = useCallback(() => {
|
||||
removeNav(modalManager.closeDetailTask);
|
||||
deepLink.handleDetailClose();
|
||||
}, [deepLink, modalManager.closeDetailTask, removeNav]);
|
||||
```
|
||||
|
||||
`removeNav` is the inverse of `pushNav` for programmatic dismissals:
|
||||
|
||||
1. Search the stack from top to bottom for the matching `close` or `revert` callback.
|
||||
2. Remove the matching entry from the internal stack.
|
||||
3. Call `window.history.back()` to consume the browser history entry.
|
||||
4. Mark the resulting `popstate` as self-triggered so the handler does not call the close/revert callback a second time.
|
||||
|
||||
The normal browser back/swipe path must **not** call `removeNav`; `popstate` already pops the stack and invokes the entry callback.
|
||||
|
||||
## Prevention
|
||||
|
||||
- Any modal or overlay view opened with `pushNav` must have its programmatic close path wired to `removeNav` with the same callback reference used by `pushNav`.
|
||||
- Keep pushed callbacks stable (`useCallback` or modal-manager callbacks). Anonymous `revert: () => ...` callbacks cannot be removed later unless they are stored in a stable variable.
|
||||
- For modal-to-modal transitions that reuse the same browser history slot, prefer `replaceCurrent` rather than `removeNav` + `pushNav`.
|
||||
- Regression coverage should include both paths:
|
||||
- normal open → popstate closes modal
|
||||
- open → programmatic close → reopen → popstate closes modal
|
||||
|
||||
## Related files
|
||||
|
||||
- `packages/dashboard/app/hooks/useNavigationHistory.ts`
|
||||
- `packages/dashboard/app/hooks/__tests__/useNavigationHistory.test.ts`
|
||||
- `packages/dashboard/app/components/__tests__/navigation-history.test.tsx`
|
||||
- `packages/dashboard/app/components/AppModals.tsx`
|
||||
- `packages/dashboard/app/App.tsx`
|
||||
@@ -381,7 +381,7 @@ function AppInner() {
|
||||
const isMobile = viewportMode === "mobile";
|
||||
|
||||
// Navigation history for browser back button (desktop + mobile).
|
||||
const { pushNav, replaceCurrent } = useNavigationHistory({ enabled: true });
|
||||
const { pushNav, replaceCurrent, removeNav } = useNavigationHistory({ enabled: true });
|
||||
|
||||
// View state must be defined before useTasks since useTasks depends on taskView for SSE gating
|
||||
const { viewMode, setViewMode, taskView, handleChangeTaskView } = useViewState({
|
||||
@@ -755,6 +755,9 @@ function AppInner() {
|
||||
|
||||
// Nodes management is an overlay view (not a modal), so it stays local to App.
|
||||
const [nodesOpen, setNodesOpen] = useState(false);
|
||||
const closeNodes = useCallback(() => {
|
||||
setNodesOpen(false);
|
||||
}, []);
|
||||
const [retryingProjects, setRetryingProjects] = useState(false);
|
||||
const [missionResumeSessionId, setMissionResumeSessionId] = useState<string | undefined>(undefined);
|
||||
const [missionTargetId, setMissionTargetId] = useState<string | undefined>(undefined);
|
||||
@@ -953,6 +956,11 @@ function AppInner() {
|
||||
void refreshAppSettings();
|
||||
}, [modalManager, refreshAppSettings]);
|
||||
|
||||
const handleSettingsCloseWithNav = useCallback(() => {
|
||||
removeNav(handleSettingsClose);
|
||||
handleSettingsClose();
|
||||
}, [handleSettingsClose, removeNav]);
|
||||
|
||||
// Redirect to board if feature-gated views are disabled.
|
||||
useEffect(() => {
|
||||
if (!settingsLoaded) return;
|
||||
@@ -990,9 +998,10 @@ function AppInner() {
|
||||
// Auto-close nodes overlay if feature flag is toggled off while overlay is open
|
||||
useEffect(() => {
|
||||
if (nodesOpen && !nodesEnabled) {
|
||||
setNodesOpen(false);
|
||||
removeNav(closeNodes);
|
||||
closeNodes();
|
||||
}
|
||||
}, [nodesOpen, nodesEnabled]);
|
||||
}, [closeNodes, nodesOpen, nodesEnabled, removeNav]);
|
||||
const {
|
||||
availableModels,
|
||||
favoriteProviders,
|
||||
@@ -1133,16 +1142,21 @@ function AppInner() {
|
||||
setNodesOpen((prev) => !prev);
|
||||
}, [nodesEnabled]);
|
||||
|
||||
const closeNodesWithNav = useCallback(() => {
|
||||
removeNav(closeNodes);
|
||||
closeNodes();
|
||||
}, [closeNodes, removeNav]);
|
||||
|
||||
// History-aware nodes toggle — pushes nav entry only when opening
|
||||
const handleOpenNodesWithNav = useCallback(() => {
|
||||
if (!nodesEnabled) return;
|
||||
if (!nodesOpen) {
|
||||
setNodesOpen(true);
|
||||
pushNav({ type: "view", revert: () => setNodesOpen(false) });
|
||||
pushNav({ type: "view", revert: closeNodes });
|
||||
} else {
|
||||
setNodesOpen(false);
|
||||
closeNodesWithNav();
|
||||
}
|
||||
}, [nodesEnabled, nodesOpen, pushNav]);
|
||||
}, [closeNodes, closeNodesWithNav, nodesEnabled, nodesOpen, pushNav]);
|
||||
|
||||
// History-aware modal open handlers — push nav entries for back-navigation.
|
||||
const openDetailTask = useCallback((task: Task | TaskDetail, tab?: Parameters<typeof modalManager.openDetailTask>[1], opts?: { origin?: DetailTaskOrigin }) => {
|
||||
@@ -1195,9 +1209,10 @@ function AppInner() {
|
||||
modalManager.toggleTerminal();
|
||||
pushNav({ type: "modal", close: modalManager.closeTerminal });
|
||||
} else {
|
||||
removeNav(modalManager.closeTerminal);
|
||||
modalManager.toggleTerminal();
|
||||
}
|
||||
}, [modalManager, pushNav]);
|
||||
}, [modalManager, pushNav, removeNav]);
|
||||
|
||||
const openFilesWithNav = useCallback((workspace?: string, initialFile?: string | null) => {
|
||||
modalManager.openFiles(workspace, initialFile);
|
||||
@@ -1408,7 +1423,7 @@ function AppInner() {
|
||||
<div className="nodes-management-overlay">
|
||||
<PageErrorBoundary>
|
||||
<Suspense fallback={null}>
|
||||
<NodesView addToast={addToast} onClose={() => setNodesOpen(false)} />
|
||||
<NodesView addToast={addToast} onClose={closeNodesWithNav} />
|
||||
</Suspense>
|
||||
</PageErrorBoundary>
|
||||
</div>
|
||||
@@ -1804,7 +1819,7 @@ function AppInner() {
|
||||
const isRevalidating = projectsLoading || currentProjectLoading || isStale;
|
||||
|
||||
return (
|
||||
<NavigationHistoryProvider value={{ pushNav, replaceCurrent }}>
|
||||
<NavigationHistoryProvider value={{ pushNav, replaceCurrent, removeNav }}>
|
||||
<FileBrowserProvider openFile={openFileInBrowser}>
|
||||
<RetryWarningProvider value={maxTotalRetriesBeforeFail * RETRY_WARNING_RATIO}>
|
||||
{isFirstEverBoot ? (
|
||||
@@ -2108,7 +2123,7 @@ function AppInner() {
|
||||
taskOperations={{ moveTask, deleteTask, mergeTask, archiveTask, retryTask, resetTask, duplicateTask }}
|
||||
deepLink={{ handleDetailClose }}
|
||||
settings={{ prAuthAvailable, themeMode, colorTheme, dashboardFontScalePct, setThemeMode, setColorTheme, setDashboardFontScalePct }}
|
||||
onSettingsClose={handleSettingsClose}
|
||||
onSettingsClose={handleSettingsCloseWithNav}
|
||||
onReopenOnboarding={reopenOnboardingWithNav}
|
||||
onOpenApprovals={(_approvalId) => handleTaskViewChange("mailbox")}
|
||||
/>
|
||||
|
||||
@@ -106,7 +106,7 @@ export function AppModals({
|
||||
onReopenOnboarding,
|
||||
onOpenApprovals,
|
||||
}: AppModalsProps) {
|
||||
const { pushNav } = useNavigationHistoryContext();
|
||||
const { pushNav, removeNav } = useNavigationHistoryContext();
|
||||
const [firstCreatedTask, setFirstCreatedTask] = useState<Task | null>(null);
|
||||
const detailTask = modalManager.detailTask
|
||||
? (() => {
|
||||
@@ -131,6 +131,101 @@ export function AppModals({
|
||||
// Use the override handler if provided, otherwise fall back to modalManager.closeSettings
|
||||
const handleSettingsClose = onSettingsClose ?? modalManager.closeSettings;
|
||||
|
||||
const closeDetailWithNav = useCallback(() => {
|
||||
removeNav(modalManager.closeDetailTask);
|
||||
deepLink.handleDetailClose();
|
||||
}, [deepLink, modalManager.closeDetailTask, removeNav]);
|
||||
|
||||
const closeGroupWithNav = useCallback(() => {
|
||||
removeNav(modalManager.closeGroupModal);
|
||||
modalManager.closeGroupModal();
|
||||
}, [modalManager.closeGroupModal, removeNav]);
|
||||
|
||||
const closeSettingsWithNav = useCallback(() => {
|
||||
removeNav(handleSettingsClose);
|
||||
handleSettingsClose();
|
||||
}, [handleSettingsClose, removeNav]);
|
||||
|
||||
const closeGitHubImportWithNav = useCallback(() => {
|
||||
removeNav(modalManager.closeGitHubImport);
|
||||
modalManager.closeGitHubImport();
|
||||
}, [modalManager.closeGitHubImport, removeNav]);
|
||||
|
||||
const closePlanningWithNav = useCallback(() => {
|
||||
removeNav(modalManager.closePlanning);
|
||||
modalManager.closePlanning();
|
||||
}, [modalManager.closePlanning, removeNav]);
|
||||
|
||||
const closeSubtaskWithNav = useCallback(() => {
|
||||
removeNav(modalManager.closeSubtask);
|
||||
modalManager.closeSubtask();
|
||||
}, [modalManager.closeSubtask, removeNav]);
|
||||
|
||||
const closeTerminalWithNav = useCallback(() => {
|
||||
removeNav(modalManager.closeTerminal);
|
||||
modalManager.closeTerminal();
|
||||
}, [modalManager.closeTerminal, removeNav]);
|
||||
|
||||
const closeScriptsWithNav = useCallback(() => {
|
||||
removeNav(modalManager.closeScripts);
|
||||
modalManager.closeScripts();
|
||||
}, [modalManager.closeScripts, removeNav]);
|
||||
|
||||
const closeFilesWithNav = useCallback(() => {
|
||||
removeNav(modalManager.closeFiles);
|
||||
modalManager.closeFiles();
|
||||
}, [modalManager.closeFiles, removeNav]);
|
||||
|
||||
const closeTodosWithNav = useCallback(() => {
|
||||
removeNav(modalManager.closeTodos);
|
||||
modalManager.closeTodos();
|
||||
}, [modalManager.closeTodos, removeNav]);
|
||||
|
||||
const closeUsageWithNav = useCallback(() => {
|
||||
removeNav(modalManager.closeUsage);
|
||||
modalManager.closeUsage();
|
||||
}, [modalManager.closeUsage, removeNav]);
|
||||
|
||||
const closeSystemStatsWithNav = useCallback(() => {
|
||||
removeNav(modalManager.closeSystemStats);
|
||||
modalManager.closeSystemStats();
|
||||
}, [modalManager.closeSystemStats, removeNav]);
|
||||
|
||||
const closeSchedulesWithNav = useCallback(() => {
|
||||
removeNav(modalManager.closeSchedules);
|
||||
modalManager.closeSchedules();
|
||||
}, [modalManager.closeSchedules, removeNav]);
|
||||
|
||||
const closeNewTaskWithNav = useCallback(() => {
|
||||
removeNav(modalManager.closeNewTask);
|
||||
modalManager.closeNewTask();
|
||||
}, [modalManager.closeNewTask, removeNav]);
|
||||
|
||||
const closeActivityLogWithNav = useCallback(() => {
|
||||
removeNav(modalManager.closeActivityLog);
|
||||
modalManager.closeActivityLog();
|
||||
}, [modalManager.closeActivityLog, removeNav]);
|
||||
|
||||
const closeGitManagerWithNav = useCallback(() => {
|
||||
removeNav(modalManager.closeGitManager);
|
||||
modalManager.closeGitManager();
|
||||
}, [modalManager.closeGitManager, removeNav]);
|
||||
|
||||
const closeWorkflowEditorWithNav = useCallback(() => {
|
||||
removeNav(modalManager.closeWorkflowEditor);
|
||||
modalManager.closeWorkflowEditor();
|
||||
}, [modalManager.closeWorkflowEditor, removeNav]);
|
||||
|
||||
const closeAgentsWithNav = useCallback(() => {
|
||||
removeNav(modalManager.closeAgents);
|
||||
modalManager.closeAgents();
|
||||
}, [modalManager.closeAgents, removeNav]);
|
||||
|
||||
const closeSetupWizardWithNav = useCallback(() => {
|
||||
removeNav(modalManager.closeSetupWizard);
|
||||
modalManager.closeSetupWizard();
|
||||
}, [modalManager.closeSetupWizard, removeNav]);
|
||||
|
||||
const handleOpenNewTask = useCallback(() => {
|
||||
modalManager.openNewTask();
|
||||
}, [modalManager]);
|
||||
@@ -190,7 +285,7 @@ export function AppModals({
|
||||
task={detailTask}
|
||||
projectId={projectId}
|
||||
tasks={tasks}
|
||||
onClose={deepLink.handleDetailClose}
|
||||
onClose={closeDetailWithNav}
|
||||
onOpenDetail={openDetailTaskWithNav}
|
||||
mobileHeaderMode={modalManager.detailTaskOrigin === "list-mobile" ? "back" : "close"}
|
||||
onMoveTask={taskOperations.moveTask}
|
||||
@@ -213,7 +308,7 @@ export function AppModals({
|
||||
<ModalErrorBoundary>
|
||||
<GroupTaskModal
|
||||
isOpen={Boolean(modalManager.groupModalGroupId)}
|
||||
onClose={modalManager.closeGroupModal}
|
||||
onClose={closeGroupWithNav}
|
||||
groupId={modalManager.groupModalGroupId}
|
||||
projectId={projectId}
|
||||
onOpenMemberTask={(taskId) => {
|
||||
@@ -230,7 +325,7 @@ export function AppModals({
|
||||
<ModalErrorBoundary>
|
||||
<Suspense fallback={null}>
|
||||
<SettingsModal
|
||||
onClose={handleSettingsClose}
|
||||
onClose={closeSettingsWithNav}
|
||||
addToast={addToast}
|
||||
initialSection={modalManager.settingsInitialSection}
|
||||
projectId={projectId}
|
||||
@@ -243,7 +338,7 @@ export function AppModals({
|
||||
onReopenOnboarding={onReopenOnboarding}
|
||||
onOpenApprovals={onOpenApprovals}
|
||||
onOpenWorkflowSettings={() => {
|
||||
handleSettingsClose();
|
||||
closeSettingsWithNav();
|
||||
modalManager.openWorkflowEditor("settings");
|
||||
}}
|
||||
/>
|
||||
@@ -253,7 +348,7 @@ export function AppModals({
|
||||
|
||||
<GitHubImportModal
|
||||
isOpen={modalManager.githubImportOpen}
|
||||
onClose={modalManager.closeGitHubImport}
|
||||
onClose={closeGitHubImportWithNav}
|
||||
onImport={taskHandlers.handleGitHubImport}
|
||||
tasks={tasks}
|
||||
projectId={projectId}
|
||||
@@ -262,7 +357,7 @@ export function AppModals({
|
||||
<ModalErrorBoundary>
|
||||
<PlanningModeModal
|
||||
isOpen={modalManager.isPlanningOpen}
|
||||
onClose={modalManager.closePlanning}
|
||||
onClose={closePlanningWithNav}
|
||||
onTaskCreated={taskHandlers.handlePlanningTaskCreated}
|
||||
onTasksCreated={taskHandlers.handlePlanningTasksCreated}
|
||||
tasks={tasks}
|
||||
@@ -275,7 +370,7 @@ export function AppModals({
|
||||
<ModalErrorBoundary>
|
||||
<SubtaskBreakdownModal
|
||||
isOpen={modalManager.isSubtaskOpen}
|
||||
onClose={modalManager.closeSubtask}
|
||||
onClose={closeSubtaskWithNav}
|
||||
initialDescription={modalManager.subtaskInitialDescription ?? ""}
|
||||
onTasksCreated={taskHandlers.handleSubtaskTasksCreated}
|
||||
projectId={projectId}
|
||||
@@ -286,14 +381,14 @@ export function AppModals({
|
||||
|
||||
<TerminalModal
|
||||
isOpen={modalManager.terminalOpen}
|
||||
onClose={modalManager.closeTerminal}
|
||||
onClose={closeTerminalWithNav}
|
||||
initialCommand={modalManager.terminalInitialCommand}
|
||||
projectId={projectId}
|
||||
/>
|
||||
|
||||
<ScriptsModal
|
||||
isOpen={modalManager.scriptsOpen}
|
||||
onClose={modalManager.closeScripts}
|
||||
onClose={closeScriptsWithNav}
|
||||
addToast={addToast}
|
||||
onRunScript={modalManager.runScript}
|
||||
projectId={projectId}
|
||||
@@ -304,7 +399,7 @@ export function AppModals({
|
||||
initialWorkspace={modalManager.fileBrowserWorkspace}
|
||||
initialFile={modalManager.fileBrowserInitialFile}
|
||||
isOpen={true}
|
||||
onClose={modalManager.closeFiles}
|
||||
onClose={closeFilesWithNav}
|
||||
onWorkspaceChange={modalManager.setFileWorkspace}
|
||||
projectId={projectId}
|
||||
/>
|
||||
@@ -313,7 +408,7 @@ export function AppModals({
|
||||
{modalManager.todosOpen && (
|
||||
<TodoModal
|
||||
isOpen={true}
|
||||
onClose={modalManager.closeTodos}
|
||||
onClose={closeTodosWithNav}
|
||||
addToast={addToast}
|
||||
projectId={projectId}
|
||||
onPlanningMode={modalManager.openPlanningWithInitialPlan}
|
||||
@@ -322,20 +417,20 @@ export function AppModals({
|
||||
|
||||
<UsageIndicator
|
||||
isOpen={modalManager.usageOpen}
|
||||
onClose={modalManager.closeUsage}
|
||||
onClose={closeUsageWithNav}
|
||||
projectId={projectId}
|
||||
anchorRect={modalManager.usageAnchorRect}
|
||||
/>
|
||||
|
||||
<SystemStatsModal
|
||||
isOpen={modalManager.systemStatsOpen}
|
||||
onClose={modalManager.closeSystemStats}
|
||||
onClose={closeSystemStatsWithNav}
|
||||
projectId={projectId}
|
||||
/>
|
||||
|
||||
{modalManager.schedulesOpen && (
|
||||
<ScheduledTasksModal
|
||||
onClose={modalManager.closeSchedules}
|
||||
onClose={closeSchedulesWithNav}
|
||||
addToast={addToast}
|
||||
projectId={projectId}
|
||||
/>
|
||||
@@ -344,7 +439,7 @@ export function AppModals({
|
||||
<ModalErrorBoundary>
|
||||
<NewTaskModal
|
||||
isOpen={modalManager.newTaskModalOpen}
|
||||
onClose={modalManager.closeNewTask}
|
||||
onClose={closeNewTaskWithNav}
|
||||
tasks={tasks}
|
||||
onCreateTask={handleModalCreateWithOnboardingTracking}
|
||||
addToast={addToast}
|
||||
@@ -354,7 +449,7 @@ export function AppModals({
|
||||
|
||||
<ActivityLogModal
|
||||
isOpen={modalManager.activityLogOpen}
|
||||
onClose={modalManager.closeActivityLog}
|
||||
onClose={closeActivityLogWithNav}
|
||||
tasks={tasks}
|
||||
projectId={projectId}
|
||||
projects={projects}
|
||||
@@ -370,7 +465,7 @@ export function AppModals({
|
||||
<ModalErrorBoundary>
|
||||
<GitManagerModal
|
||||
isOpen={modalManager.gitManagerOpen}
|
||||
onClose={modalManager.closeGitManager}
|
||||
onClose={closeGitManagerWithNav}
|
||||
tasks={tasks}
|
||||
addToast={addToast}
|
||||
projectId={projectId}
|
||||
@@ -382,7 +477,7 @@ export function AppModals({
|
||||
<Suspense fallback={null}>
|
||||
<WorkflowNodeEditor
|
||||
isOpen={modalManager.workflowEditorOpen}
|
||||
onClose={modalManager.closeWorkflowEditor}
|
||||
onClose={closeWorkflowEditorWithNav}
|
||||
addToast={addToast}
|
||||
projectId={projectId}
|
||||
initialPanel={modalManager.workflowEditorInitialPanel}
|
||||
@@ -395,7 +490,7 @@ export function AppModals({
|
||||
|
||||
<AgentListModal
|
||||
isOpen={modalManager.agentsOpen}
|
||||
onClose={modalManager.closeAgents}
|
||||
onClose={closeAgentsWithNav}
|
||||
addToast={addToast}
|
||||
projectId={projectId}
|
||||
/>
|
||||
@@ -404,7 +499,7 @@ export function AppModals({
|
||||
<Suspense fallback={null}>
|
||||
<SetupWizardModal
|
||||
onProjectRegistered={projectActions.handleSetupComplete}
|
||||
onClose={modalManager.closeSetupWizard}
|
||||
onClose={closeSetupWizardWithNav}
|
||||
/>
|
||||
</Suspense>
|
||||
)}
|
||||
|
||||
@@ -558,10 +558,13 @@ describe("AppModals", () => {
|
||||
expect(mockSystemStatsModalProps).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
isOpen: true,
|
||||
onClose: closeSystemStats,
|
||||
onClose: expect.any(Function),
|
||||
projectId: "proj-system",
|
||||
}),
|
||||
);
|
||||
|
||||
mockSystemStatsModalProps.mock.calls[0][0].onClose();
|
||||
expect(closeSystemStats).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -9,7 +9,7 @@
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { render, screen, fireEvent, waitFor, act } from "@testing-library/react";
|
||||
import type { Settings } from "@fusion/core";
|
||||
import type { Settings, Task } from "@fusion/core";
|
||||
import type { ProjectInfo } from "../../api";
|
||||
import { scopedKey } from "../../utils/projectStorage";
|
||||
|
||||
@@ -144,6 +144,18 @@ vi.mock("../../components/model-onboarding-state", () => ({
|
||||
ONBOARDING_FLOW_STEPS: ["ai-setup", "github", "project-setup", "first-task"],
|
||||
}));
|
||||
|
||||
vi.mock("../../components/Board", () => ({
|
||||
Board: ({ tasks, onOpenDetail }: { tasks: Task[]; onOpenDetail: (task: Task) => void }) => (
|
||||
<div data-testid="board-view">
|
||||
{tasks.map((task) => (
|
||||
<button key={task.id} type="button" data-testid={`open-task-${task.id}`} onClick={() => onOpenDetail(task)}>
|
||||
{task.title}
|
||||
</button>
|
||||
))}
|
||||
</div>
|
||||
),
|
||||
}));
|
||||
|
||||
vi.mock("../../components/TaskDetailModal", () => ({
|
||||
TaskDetailModal: ({ task, onClose }: { task: { id: string; title?: string }; onClose: () => void }) => (
|
||||
<div className="modal-overlay open" data-testid="task-detail-modal">
|
||||
@@ -305,6 +317,18 @@ vi.mock("../../hooks/useMobileKeyboard", () => ({
|
||||
|
||||
import { App } from "../../App";
|
||||
|
||||
function makeTask(id: string, title: string): Task {
|
||||
return {
|
||||
id,
|
||||
title,
|
||||
description: "Test task description",
|
||||
column: "todo",
|
||||
status: "todo",
|
||||
createdAt: new Date(0).toISOString(),
|
||||
updatedAt: new Date(0).toISOString(),
|
||||
} as Task;
|
||||
}
|
||||
|
||||
function dispatchPopState(state: Record<string, unknown> | null) {
|
||||
act(() => {
|
||||
window.dispatchEvent(new PopStateEvent("popstate", { state }));
|
||||
@@ -380,6 +404,14 @@ describe("Navigation history integration", () => {
|
||||
return result;
|
||||
}
|
||||
|
||||
async function renderMobileAppAndWait() {
|
||||
const result = render(<App />);
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("board-view")).toBeTruthy();
|
||||
});
|
||||
return result;
|
||||
}
|
||||
|
||||
// 1. Desktop: opening Settings pushes a history entry
|
||||
it("pushes history entry when opening Settings modal on desktop", async () => {
|
||||
await renderAppAndWait();
|
||||
@@ -481,6 +513,85 @@ describe("Navigation history integration", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("dismisses task detail on mobile popstate after a normal open", async () => {
|
||||
mockUseViewportMode.mockReturnValue("mobile");
|
||||
const task = makeTask("FN-1", "Mobile Swipe Detail");
|
||||
mockUseTasks.mockImplementation(() => ({
|
||||
tasks: [task],
|
||||
createTask: mockCreateTask,
|
||||
moveTask: vi.fn(),
|
||||
deleteTask: vi.fn(),
|
||||
mergeTask: vi.fn(),
|
||||
retryTask: vi.fn(),
|
||||
updateTask: vi.fn(),
|
||||
duplicateTask: vi.fn(),
|
||||
archiveTask: vi.fn(),
|
||||
unarchiveTask: vi.fn(),
|
||||
archiveAllDone: vi.fn(),
|
||||
}));
|
||||
|
||||
await renderMobileAppAndWait();
|
||||
|
||||
fireEvent.click(screen.getByTestId("open-task-FN-1"));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("task-detail-modal")).toBeTruthy();
|
||||
});
|
||||
|
||||
dispatchPopState({ navIndex: 0 });
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByTestId("task-detail-modal")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
it("dismisses task detail on mobile popstate after close and reopen", async () => {
|
||||
mockUseViewportMode.mockReturnValue("mobile");
|
||||
const task = makeTask("FN-1", "Mobile Swipe Detail");
|
||||
mockUseTasks.mockImplementation(() => ({
|
||||
tasks: [task],
|
||||
createTask: mockCreateTask,
|
||||
moveTask: vi.fn(),
|
||||
deleteTask: vi.fn(),
|
||||
mergeTask: vi.fn(),
|
||||
retryTask: vi.fn(),
|
||||
updateTask: vi.fn(),
|
||||
duplicateTask: vi.fn(),
|
||||
archiveTask: vi.fn(),
|
||||
unarchiveTask: vi.fn(),
|
||||
archiveAllDone: vi.fn(),
|
||||
}));
|
||||
|
||||
await renderMobileAppAndWait();
|
||||
|
||||
fireEvent.click(screen.getByTestId("open-task-FN-1"));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("task-detail-modal")).toBeTruthy();
|
||||
});
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: "Close" }));
|
||||
// removeNav drives history.back(); consume the self-triggered popstate
|
||||
// before reopening so the next popstate represents the user's swipe-back.
|
||||
dispatchPopState({ navIndex: 0 });
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByTestId("task-detail-modal")).toBeNull();
|
||||
});
|
||||
|
||||
fireEvent.click(screen.getByTestId("open-task-FN-1"));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("task-detail-modal")).toBeTruthy();
|
||||
});
|
||||
|
||||
dispatchPopState({ navIndex: 0 });
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByTestId("task-detail-modal")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
// 5. Verify useNavigationHistory is called with enabled=true on mobile
|
||||
it("calls useNavigationHistory with enabled=true on mobile", async () => {
|
||||
mockUseViewportMode.mockReturnValue("mobile");
|
||||
|
||||
@@ -11,25 +11,30 @@ import {
|
||||
describe("useNavigationHistory", () => {
|
||||
const originalPushState = window.history.pushState;
|
||||
const originalReplaceState = window.history.replaceState;
|
||||
const originalBack = window.history.back;
|
||||
|
||||
let pushStateSpy: ReturnType<typeof vi.fn>;
|
||||
let replaceStateSpy: ReturnType<typeof vi.fn>;
|
||||
let backSpy: ReturnType<typeof vi.fn>;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
pushStateSpy = vi.fn();
|
||||
replaceStateSpy = vi.fn();
|
||||
backSpy = vi.fn();
|
||||
// Use real replaceState for setup so history.state is actually set,
|
||||
// then install spies for assertions.
|
||||
window.history.replaceState = originalReplaceState;
|
||||
window.history.replaceState({}, "");
|
||||
window.history.pushState = pushStateSpy;
|
||||
window.history.replaceState = replaceStateSpy;
|
||||
window.history.back = backSpy;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
window.history.pushState = originalPushState;
|
||||
window.history.replaceState = originalReplaceState;
|
||||
window.history.back = originalBack;
|
||||
});
|
||||
|
||||
function renderHookWithHistory(enabled = true) {
|
||||
@@ -128,6 +133,98 @@ describe("useNavigationHistory", () => {
|
||||
expect(close1).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("removeNav removes a matching entry and calls history.back", () => {
|
||||
const close = vi.fn();
|
||||
const { result } = renderHookWithHistory();
|
||||
|
||||
act(() => {
|
||||
result.current.pushNav({ type: "modal", close });
|
||||
});
|
||||
|
||||
act(() => {
|
||||
result.current.removeNav(close);
|
||||
});
|
||||
|
||||
expect(backSpy).toHaveBeenCalledTimes(1);
|
||||
|
||||
dispatchPopState({ navIndex: 0 });
|
||||
|
||||
expect(close).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("consumes the self-triggered popstate after removeNav without invoking callbacks", () => {
|
||||
const close = vi.fn();
|
||||
const revert = vi.fn();
|
||||
const { result } = renderHookWithHistory();
|
||||
|
||||
act(() => {
|
||||
result.current.pushNav({ type: "modal", close });
|
||||
result.current.pushNav({ type: "view", revert });
|
||||
result.current.removeNav(revert);
|
||||
});
|
||||
|
||||
dispatchPopState({ navIndex: 1 });
|
||||
|
||||
expect(revert).not.toHaveBeenCalled();
|
||||
expect(close).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("allows reopening with the same callback after removeNav", () => {
|
||||
const close = vi.fn();
|
||||
const { result } = renderHookWithHistory();
|
||||
|
||||
act(() => {
|
||||
result.current.pushNav({ type: "modal", close });
|
||||
result.current.removeNav(close);
|
||||
});
|
||||
|
||||
dispatchPopState({ navIndex: 0 });
|
||||
|
||||
act(() => {
|
||||
result.current.pushNav({ type: "modal", close });
|
||||
});
|
||||
|
||||
expect(pushStateSpy).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("keeps stack order consistent after removing the top entry", () => {
|
||||
const close1 = vi.fn();
|
||||
const close2 = vi.fn();
|
||||
const close3 = vi.fn();
|
||||
const { result } = renderHookWithHistory();
|
||||
|
||||
act(() => {
|
||||
result.current.pushNav({ type: "modal", close: close1 });
|
||||
result.current.pushNav({ type: "modal", close: close2 });
|
||||
result.current.pushNav({ type: "modal", close: close3 });
|
||||
result.current.removeNav(close3);
|
||||
});
|
||||
|
||||
dispatchPopState({ navIndex: 2 });
|
||||
dispatchPopState({ navIndex: 1 });
|
||||
|
||||
expect(close3).not.toHaveBeenCalled();
|
||||
expect(close2).toHaveBeenCalledTimes(1);
|
||||
expect(close1).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("removeNav is a no-op when the callback is not on the stack", () => {
|
||||
const close = vi.fn();
|
||||
const absent = vi.fn();
|
||||
const { result } = renderHookWithHistory();
|
||||
|
||||
act(() => {
|
||||
result.current.pushNav({ type: "modal", close });
|
||||
result.current.removeNav(absent);
|
||||
});
|
||||
|
||||
expect(backSpy).not.toHaveBeenCalled();
|
||||
|
||||
dispatchPopState({ navIndex: 0 });
|
||||
|
||||
expect(close).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// 5. replaceCurrent updates the top entry
|
||||
it("replaceCurrent updates the top entry", () => {
|
||||
const closeA = vi.fn();
|
||||
@@ -325,6 +422,7 @@ describe("useNavigationHistory", () => {
|
||||
const value: UseNavigationHistoryResult = {
|
||||
pushNav: vi.fn(),
|
||||
replaceCurrent: vi.fn(),
|
||||
removeNav: vi.fn(),
|
||||
};
|
||||
|
||||
const wrapper = ({ children }: { children: ReactNode }) =>
|
||||
|
||||
@@ -33,8 +33,15 @@ export interface UseNavigationHistoryResult {
|
||||
pushNav: (entry: NavEntry) => void;
|
||||
/** Replace the top-of-stack entry and call history.replaceState. */
|
||||
replaceCurrent: (entry: NavEntry) => void;
|
||||
/**
|
||||
* Remove a programmatically-dismissed entry and call history.back so the
|
||||
* browser history entry created by pushNav is consumed as well.
|
||||
*/
|
||||
removeNav: (closeOrRevert: () => void) => void;
|
||||
}
|
||||
|
||||
const SELF_POP_FALLBACK_CLEAR_MS = 1_000;
|
||||
|
||||
export const NavigationHistoryContext = createContext<UseNavigationHistoryResult | null>(null);
|
||||
|
||||
export function NavigationHistoryProvider({
|
||||
@@ -76,6 +83,12 @@ export function useNavigationHistory(
|
||||
// history entry. pushNav checks this flag and skips if true.
|
||||
const isPoppingRef = useRef(false);
|
||||
|
||||
// Guard flag: removeNav calls history.back() to consume the matching browser
|
||||
// history entry after the UI has already closed. The resulting popstate must
|
||||
// be ignored so callbacks are not invoked twice.
|
||||
const selfPopRef = useRef(false);
|
||||
const selfPopClearTimerRef = useRef<number | null>(null);
|
||||
|
||||
// Keep enabled in a ref so the popstate handler can read the current value
|
||||
// without needing to be re-registered on every change.
|
||||
const enabledRef = useRef(enabled);
|
||||
@@ -128,6 +141,33 @@ export function useNavigationHistory(
|
||||
[], // stable — reads from refs
|
||||
);
|
||||
|
||||
const removeNav = useCallback(
|
||||
(closeOrRevert: () => void) => {
|
||||
if (!enabledRef.current) return;
|
||||
|
||||
for (let i = stackRef.current.length - 1; i >= 0; i -= 1) {
|
||||
const entry = stackRef.current[i];
|
||||
const callback = entry.type === "modal" ? entry.close : entry.revert;
|
||||
if (callback !== closeOrRevert) continue;
|
||||
|
||||
stackRef.current.splice(i, 1);
|
||||
selfPopRef.current = true;
|
||||
|
||||
if (selfPopClearTimerRef.current !== null) {
|
||||
window.clearTimeout(selfPopClearTimerRef.current);
|
||||
}
|
||||
selfPopClearTimerRef.current = window.setTimeout(() => {
|
||||
selfPopRef.current = false;
|
||||
selfPopClearTimerRef.current = null;
|
||||
}, SELF_POP_FALLBACK_CLEAR_MS);
|
||||
|
||||
window.history.back();
|
||||
return;
|
||||
}
|
||||
},
|
||||
[], // stable — reads from refs
|
||||
);
|
||||
|
||||
// Register popstate listener. Always registers in browser environments but
|
||||
// the handler checks enabledRef.current to skip when disabled (desktop).
|
||||
useEffect(() => {
|
||||
@@ -136,6 +176,15 @@ export function useNavigationHistory(
|
||||
const handlePopState = (event: PopStateEvent) => {
|
||||
if (!enabledRef.current) return;
|
||||
|
||||
if (selfPopRef.current) {
|
||||
selfPopRef.current = false;
|
||||
if (selfPopClearTimerRef.current !== null) {
|
||||
window.clearTimeout(selfPopClearTimerRef.current);
|
||||
selfPopClearTimerRef.current = null;
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
const targetIndex = event.state?.navIndex ?? 0;
|
||||
const currentLength = stackRef.current.length;
|
||||
|
||||
@@ -168,8 +217,12 @@ export function useNavigationHistory(
|
||||
window.addEventListener("popstate", handlePopState);
|
||||
return () => {
|
||||
window.removeEventListener("popstate", handlePopState);
|
||||
if (selfPopClearTimerRef.current !== null) {
|
||||
window.clearTimeout(selfPopClearTimerRef.current);
|
||||
selfPopClearTimerRef.current = null;
|
||||
}
|
||||
};
|
||||
}, []);
|
||||
|
||||
return { pushNav, replaceCurrent };
|
||||
return { pushNav, replaceCurrent, removeNav };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user