Merge pull request 'Make mobile task overlays browser-back safe' (#316) from timmy/315-mobile-overlay-back-safety into main

2026-08-08 15:56:38 +00:00 · 2026-08-08 15:56:38 +00:00 · 6143568b95
commit 6143568b95
parent 80a1fa60bd 9a152c435e
5 changed files with 262 additions and 17 deletions
--- a/frontend/dashboard.js
+++ b/frontend/dashboard.js
@ -1358,7 +1358,7 @@
          const claimed = lastMyWork.find(work =>
            work.kind === 'issue' && work.repository === confirmed.repository && work.number === confirmed.number
          );
-          closeFindWorkSheet();
+          taskOverlayHistory.leave();
          refreshMyWorkView();
          qs('#my-work-action-status').textContent = confirmed.repository + '#' + confirmed.number + ' assigned to you.';
          openRoutedWork(claimed, qs('#find-work'));
@ -1371,7 +1371,11 @@
    });
  }

-  async function openFindWorkSheet() {
+  async function openFindWorkSheet(navigate = true) {
+    if (navigate) {
+      taskOverlayHistory.open('find');
+      return;
+    }
    findingWork = true;
    qs('#find-work-sheet').classList.add('open');
    const retainedItems = findWorkController.items();
@ -1393,7 +1397,11 @@
    }
  }

-  function closeFindWorkSheet() {
+  function closeFindWorkSheet(navigate = true) {
+    if (navigate && taskOverlayHistory.current() === 'find') {
+      taskOverlayHistory.close();
+      return;
+    }
    findingWork = false;
    qs('#find-work-sheet').classList.remove('open');
    qs('#find-work').focus();
@ -1459,7 +1467,7 @@

  function clearSharedLaunchUrl() {
    const cleanUrl = location.pathname + location.hash;
-    history.replaceState({}, '', cleanUrl);
+    history.replaceState(history.state || {}, '', cleanUrl);
  }

  function openStagedSharedContent() {
@ -1476,7 +1484,11 @@
    sharedLaunchState = null;
  }

-  function openCreateIssueSheet() {
+  function openCreateIssueSheet(navigate = true) {
+    if (navigate) {
+      taskOverlayHistory.open('new');
+      return;
+    }
    const captureDraft = issueCapture.loadDraft();
    const repositories = (lastContextSnapshot?.repos || []).map(repository => repository.full_name).filter(Boolean);
    if (captureDraft.repository && !repositories.includes(captureDraft.repository)) {
@ -1498,7 +1510,13 @@
    qs('#create-issue-title').focus();
  }

-  function closeCreateIssueSheet() {
+  let suppressCreateDraftOnHistoryClose = false;
+  function closeCreateIssueSheet(navigate = true, preserveDraft = true) {
+    if (navigate && taskOverlayHistory.current() === 'new') {
+      suppressCreateDraftOnHistoryClose = !preserveDraft;
+      taskOverlayHistory.close();
+      return;
+    }
    qs('#create-issue-sheet').classList.remove('open');
    creatingIssue = false;
    qs('#new-issue').focus();
@ -1964,7 +1982,11 @@
    ),
    onState: renderSearchPreview,
  });
-  function closeSearchPreview() {
+  function closeSearchPreview(navigate = true) {
+    if (navigate && taskOverlayHistory.current() === 'search-preview') {
+      taskOverlayHistory.close();
+      return;
+    }
    searchPreview.close();
    qs('#cmd-palette').classList.add('open');
    qs('#cmd-input').setAttribute('aria-expanded', 'true');
@ -1977,16 +1999,18 @@
      candidate.kind === 'issue' && candidate.key === detail.repository + '#' + detail.number
    );
    if (!item) return false;
-    searchPreview.close();
+    taskOverlayHistory.leave();
    openRoutedWork(item, qs('#find-work'));
    return true;
  }
  function runCommandItem(item) {
    if (item.command) {
+      taskOverlayHistory.leave();
      item.command.run();
      qs('#cmd-input').value = '';
    } else {
      searchPreview.open(item.result).catch(() => {});
+      taskOverlayHistory.open('search-preview');
    }
    qs('#cmd-palette').classList.remove('open');
    qs('#cmd-input').setAttribute('aria-expanded', 'false');
@ -2015,13 +2039,42 @@
      item.addEventListener('click', () => runCommandItem(commandItems[Number(item.dataset.idx)]));
    });
  }
-  function openCommandPalette() {
+  function openCommandPalette(navigate = true) {
+    if (navigate) {
+      taskOverlayHistory.open('search');
+      return;
+    }
    qs('#cmd-palette').classList.add('open');
    qs('#cmd-input').setAttribute('aria-expanded', 'true');
    qs('#cmd-input').focus();
    commandSelection = -1;
    renderCommands(qs('#cmd-input').value);
  }
+  const taskOverlayHistory = createTaskOverlayHistory({
+    history: window.history,
+    eventTarget: window,
+    onChange(kind, previous) {
+      if (previous === 'new' && kind !== 'new') {
+        if (!suppressCreateDraftOnHistoryClose) saveIssueCaptureDraft();
+        suppressCreateDraftOnHistoryClose = false;
+        closeCreateIssueSheet(false);
+      }
+      if (previous === 'find' && kind !== 'find') closeFindWorkSheet(false);
+      if (previous === 'search-preview' && kind !== 'search-preview') {
+        if (kind === 'search') closeSearchPreview(false);
+        else searchPreview.close();
+      }
+      if (previous === 'search' && kind !== 'search' && kind !== 'search-preview') {
+        qs('#cmd-palette').classList.remove('open');
+        qs('#cmd-input').setAttribute('aria-expanded', 'false');
+        qs('#open-palette').focus();
+      }
+      if (kind === 'new' && previous !== 'new') openCreateIssueSheet(false);
+      if (kind === 'find' && previous !== 'find') openFindWorkSheet(false);
+      if (kind === 'search' && previous !== 'search-preview') openCommandPalette(false);
+    },
+  });
+  taskOverlayHistory.start();
  qs('#open-palette').addEventListener('click', openCommandPalette);
  qs('#cmd-input').addEventListener('input', (e) => {
    commandSelection = -1;
@ -2043,8 +2096,7 @@
  document.addEventListener('keydown', (e) => {
    if (e.key === 'Escape' && qs('#cmd-palette').classList.contains('open')) {
      e.preventDefault();
-      qs('#cmd-palette').classList.remove('open');
-      qs('#cmd-input').setAttribute('aria-expanded', 'false');
+      taskOverlayHistory.close();
      return;
    }
    if (e.key === 'Escape' && qs('#search-preview').classList.contains('open')) {
@ -2086,8 +2138,7 @@
    if ((e.metaKey||e.ctrlKey) && e.key==='k') {
      e.preventDefault();
      if (qs('#cmd-palette').classList.contains('open')) {
-        qs('#cmd-palette').classList.remove('open');
-        qs('#cmd-input').setAttribute('aria-expanded', 'false');
+        taskOverlayHistory.close();
      } else openCommandPalette();
    }
  });
@ -2144,7 +2195,7 @@
      issueCapture.clearDraft();
      qs('#create-issue-title').value = '';
      qs('#create-issue-body').value = '';
-      closeCreateIssueSheet();
+      closeCreateIssueSheet(true, false);
      refreshMyWorkView();
      qs('#my-work-action-status').textContent = 'Saved in Drafts · choose a repository after reconnecting.';
    } catch (error) {
@ -2169,11 +2220,12 @@
    qs('#create-issue-title').focus();
  });
  qs('#cancel-new-issue').addEventListener('click', () => {
+    const discardEditedDraft = Boolean(editingOutboxId);
    if (editingOutboxId) {
      issueCapture.clearDraft();
      editingOutboxId = null;
    } else saveIssueCaptureDraft();
-    closeCreateIssueSheet();
+    closeCreateIssueSheet(true, !discardEditedDraft);
  });
  ['#create-issue-title', '#create-issue-body', '#create-issue-due-date'].forEach(selector =>
    qs(selector).addEventListener('input', saveIssueCaptureDraft)
@ -2216,7 +2268,8 @@
      }
      editingOutboxId = null;
      issueCapture.clearDraft();
-      closeCreateIssueSheet();
+      suppressCreateDraftOnHistoryClose = true;
+      taskOverlayHistory.leave();
      refreshMyWorkView();
      qs('#my-work-action-status').textContent = 'Queued for sync.';
      if (navigator.onLine) applyOutboxResult(await issueOutbox.retry(queued.id, activeFlushLogin), true);
--- a/frontend/index.html
+++ b/frontend/index.html
@ -524,6 +524,7 @@
 <script src="static/pull-sheet.js"></script>
 <script src="static/review-sheet.js"></script>
 <script src="static/work-route.js"></script>
+<script src="static/task-overlay-history.js"></script>
 <script src="static/context-poller.js"></script>
 <script src="static/mobile-task-dock.js"></script>
 <script src="static/mobile-launch.js"></script>
--- a/frontend/task-overlay-history.js
+++ b/frontend/task-overlay-history.js
@ -0,0 +1,60 @@
+(function (root, factory) {
+  const api = factory();
+  if (typeof module === 'object' && module.exports) module.exports = api;
+  else root.createTaskOverlayHistory = api;
+})(typeof globalThis !== 'undefined' ? globalThis : this, function () {
+  'use strict';
+
+  const allowed = new Set(['new', 'find', 'search', 'search-preview']);
+
+  return function createTaskOverlayHistory({ history, eventTarget, onChange }) {
+    let active = allowed.has(history.state?.taskOverlay) ? history.state.taskOverlay : null;
+    let started = false;
+
+    function stateKind(state = history.state) {
+      return allowed.has(state?.taskOverlay) ? state.taskOverlay : null;
+    }
+
+    function apply(state) {
+      const next = stateKind(state);
+      if (next === active) return;
+      const previous = active;
+      active = next;
+      onChange(next, previous);
+    }
+
+    return {
+      start() {
+        if (started) return;
+        started = true;
+        eventTarget.addEventListener('popstate', event => apply(event.state));
+      },
+      open(kind) {
+        if (!allowed.has(kind)) return false;
+        if (active === kind) return true;
+        const previous = active;
+        const state = { ...(history.state || {}), taskOverlay: kind };
+        history.pushState(state, '');
+        active = kind;
+        onChange(kind, previous);
+        return true;
+      },
+      close() {
+        if (!active) return false;
+        history.back();
+        return true;
+      },
+      leave() {
+        if (!active) return false;
+        const previous = active;
+        const state = { ...(history.state || {}) };
+        delete state.taskOverlay;
+        history.replaceState(state, '');
+        active = null;
+        onChange(null, previous);
+        return true;
+      },
+      current() { return active; },
+    };
+  };
+});
--- a/tests/test_my_work.py
+++ b/tests/test_my_work.py
@ -664,7 +664,7 @@ async def test_dashboard_launches_share_capture_with_draft_conflict_choices():
    assert 'id="resume-issue-draft"' in html
    assert 'id="shared-content-conflict"' in html
    assert '.shared-content-actions button { min-height:44px;' in html
-    assert "history.replaceState({}, '', cleanUrl)" in html
+    assert "history.replaceState(history.state || {}, '', cleanUrl)" in html
    assert "navigator.serviceWorker.register('service-worker.js')" in html


--- a/tests/test_task_overlay_history.py
+++ b/tests/test_task_overlay_history.py
@ -0,0 +1,131 @@
+import json
+import subprocess
+from pathlib import Path
+
+from tests.dashboard_bundle import dashboard_bundle_text
+
+
+OVERLAY_HISTORY = Path(__file__).parents[1] / "frontend" / "task-overlay-history.js"
+
+
+def test_task_overlay_history_unwinds_preview_search_and_dashboard_without_duplicate_entries():
+    script = f"""
+const createTaskOverlayHistory = require({json.dumps(str(OVERLAY_HISTORY))});
+const listeners = {{}};
+const location = {{ hash:'', pathname:'/dashboard/' }};
+const stack = [{{ page:'dashboard' }}];
+let cursor = 0;
+const changes = [];
+const history = {{
+  get state() {{ return stack[cursor]; }},
+  pushState(state) {{ stack.splice(cursor + 1); stack.push(state); cursor += 1; }},
+  back() {{ if (cursor > 0) cursor -= 1; listeners.popstate({{state:stack[cursor]}}); }},
+}};
+const controller = createTaskOverlayHistory({{
+  history, location,
+  eventTarget: {{ addEventListener(name, callback) {{ listeners[name] = callback; }} }},
+  onChange(kind, previous) {{ changes.push([kind, previous]); }},
+}});
+controller.start();
+controller.open('search');
+controller.open('search');
+controller.open('search-preview');
+controller.close();
+controller.close();
+process.stdout.write(JSON.stringify({{changes, stack, cursor, current:controller.current()}}));
+"""
+    result = subprocess.run(
+        ["node", "-e", script], capture_output=True, text=True
+    )
+
+    assert result.returncode == 0, result.stderr
+    assert json.loads(result.stdout) == {
+        "changes": [
+            ["search", None],
+            ["search-preview", "search"],
+            ["search", "search-preview"],
+            [None, "search"],
+        ],
+        "stack": [
+            {"page": "dashboard"},
+            {"page": "dashboard", "taskOverlay": "search"},
+            {"page": "dashboard", "taskOverlay": "search-preview"},
+        ],
+        "cursor": 0,
+        "current": None,
+    }
+
+
+def test_task_overlay_history_rejects_unknown_states_and_preserves_existing_history_state():
+    script = f"""
+const createTaskOverlayHistory = require({json.dumps(str(OVERLAY_HISTORY))});
+const pushed = [];
+const history = {{
+  state: {{ workRoute:'#/my-work/issue/stackchain/api/7' }},
+  pushState(state) {{ this.state = state; pushed.push(state); }}, back() {{}},
+}};
+const controller = createTaskOverlayHistory({{
+  history, location:{{}}, eventTarget:{{addEventListener() {{}}}}, onChange() {{}},
+}});
+const invalid = controller.open('settings');
+const valid = controller.open('new');
+process.stdout.write(JSON.stringify({{invalid, valid, pushed}}));
+"""
+    result = subprocess.run(
+        ["node", "-e", script], capture_output=True, text=True
+    )
+
+    assert result.returncode == 0, result.stderr
+    assert json.loads(result.stdout) == {
+        "invalid": False,
+        "valid": True,
+        "pushed": [{
+            "workRoute": "#/my-work/issue/stackchain/api/7",
+            "taskOverlay": "new",
+        }],
+    }
+
+
+def test_task_overlay_history_can_leave_an_overlay_for_a_work_route_transition():
+    script = f"""
+const createTaskOverlayHistory = require({json.dumps(str(OVERLAY_HISTORY))});
+const changes = [];
+const history = {{
+  state: {{ page:'dashboard' }},
+  pushState(state) {{ this.state = state; }},
+  replaceState(state) {{ this.state = state; }},
+  back() {{ throw new Error('route transitions must not race history.back'); }},
+}};
+const controller = createTaskOverlayHistory({{
+  history, eventTarget:{{addEventListener() {{}}}},
+  onChange(kind, previous) {{ changes.push([kind, previous]); }},
+}});
+controller.open('find');
+const left = controller.leave();
+process.stdout.write(JSON.stringify({{left, state:history.state, current:controller.current(), changes}}));
+"""
+    result = subprocess.run(
+        ["node", "-e", script], capture_output=True, text=True
+    )
+
+    assert result.returncode == 0, result.stderr
+    assert json.loads(result.stdout) == {
+        "left": True,
+        "state": {"page": "dashboard"},
+        "current": None,
+        "changes": [["find", None], [None, "find"]],
+    }
+
+
+def test_dashboard_routes_mobile_task_overlays_through_browser_history():
+    html = dashboard_bundle_text()
+
+    assert '<script src="static/task-overlay-history.js"></script>' in html
+    assert "createTaskOverlayHistory({" in html
+    assert "taskOverlayHistory.open('new')" in html
+    assert "taskOverlayHistory.open('find')" in html
+    assert "taskOverlayHistory.open('search')" in html
+    assert "taskOverlayHistory.open('search-preview')" in html
+    assert "taskOverlayHistory.close()" in html
+    assert "saveIssueCaptureDraft();" in html
+    assert "history.replaceState(history.state || {}, '', cleanUrl)" in html