From 9a152c435e570aa5da430b48dc368470f2fee794 Mon Sep 17 00:00:00 2001 From: timmy Date: Sat, 8 Aug 2026 15:55:17 +0000 Subject: [PATCH] feat: make mobile task overlays back-safe (#315) --- frontend/dashboard.js | 85 +++++++++++++++---- frontend/index.html | 1 + frontend/task-overlay-history.js | 60 +++++++++++++ tests/test_my_work.py | 2 +- tests/test_task_overlay_history.py | 131 +++++++++++++++++++++++++++++ 5 files changed, 262 insertions(+), 17 deletions(-) create mode 100644 frontend/task-overlay-history.js create mode 100644 tests/test_task_overlay_history.py diff --git a/frontend/dashboard.js b/frontend/dashboard.js index e393dc2..f569ccf 100644 --- 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); diff --git a/frontend/index.html b/frontend/index.html index 0203188..8bbb040 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -524,6 +524,7 @@ + diff --git a/frontend/task-overlay-history.js b/frontend/task-overlay-history.js new file mode 100644 index 0000000..70fb571 --- /dev/null +++ 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; }, + }; + }; +}); diff --git a/tests/test_my_work.py b/tests/test_my_work.py index c03557b..6f6fe82 100644 --- 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 diff --git a/tests/test_task_overlay_history.py b/tests/test_task_overlay_history.py new file mode 100644 index 0000000..3a6c1fa --- /dev/null +++ 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 '' 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