From 3fde7cf3adb85033e913d84d1f8a526fe121b746 Mon Sep 17 00:00:00 2001 From: timmy Date: Mon, 17 Aug 2026 10:42:19 +0000 Subject: [PATCH] fix: foreground mobile delivery attention handoffs (Closes #1018) --- frontend/dashboard.js | 16 +-- frontend/mobile-delivery-recovery.js | 12 +- .../e2e/test_mobile_home_bootstrap_release.py | 34 ++++++ tests/test_frontend_bundle.py | 2 + tests/test_mobile_delivery_recovery.py | 106 +++++++++++++++++- 5 files changed, 156 insertions(+), 14 deletions(-) diff --git a/frontend/dashboard.js b/frontend/dashboard.js index 4905a8b..4155af6 100644 --- a/frontend/dashboard.js +++ b/frontend/dashboard.js @@ -3205,14 +3205,14 @@ const button = event.currentTarget; if (!activeFlushLogin || !deliveryCenter.retryable.length) return; button.disabled = true; - qs('#my-work-action-status').textContent = 'Retrying safe waiting deliveries…'; + qs('#my-work-action-status').textContent = 'Retrying waiting deliveries…'; const [issueResult, authoredResult] = await Promise.all([issueOutbox.flush(activeFlushLogin), authoredOutbox.flush(activeFlushLogin)]); applyOutboxResult(issueResult); applyAuthoredOutboxResult(authoredResult); - qs('#my-work-action-status').textContent = 'Waiting deliveries retried. Items needing attention were skipped.'; + qs('#my-work-action-status').textContent = 'Waiting deliveries retried; attention stayed queued.'; }); list.querySelectorAll('.draft-resume').forEach(button => { - button.addEventListener('click', async () => { + button.onclick=async()=>{ const item = lastDrafts[Number(button.dataset.draftIndex)]; if (!item) return; if (item.kind === 'unfiled-issue') { @@ -3221,7 +3221,7 @@ } catch (error) { qs('#my-work-action-status').textContent = error.message; } } else if (item.kind === 'new-issue') openCreateIssueSheet(); else if (item.route) workRoute.open(item.route); - }); + }; }); list.querySelectorAll('.draft-continue').forEach(button => { button.addEventListener('click', () => { @@ -3268,7 +3268,7 @@ }); }); list.querySelectorAll('.draft-review-checklist').forEach(button => { - button.addEventListener('click', async () => { + button.onclick=async()=>{ const item = lastDrafts[Number(button.dataset.draftIndex)]; const queued = authoredOutbox.list().find(candidate => candidate.id === item?.outbox_id); if (!item?.checklist_conflict || !queued || !activeFlushLogin) return; @@ -3301,7 +3301,7 @@ } finally { button.disabled = false; } - }); + }; }); list.querySelectorAll('.draft-authorize').forEach(button => { button.addEventListener('click', async () => { @@ -3322,12 +3322,12 @@ }); }); list.querySelectorAll('.draft-copy').forEach(button => { - button.addEventListener('click', async () => { + button.onclick=async()=>{ const item = lastDrafts[Number(button.dataset.draftIndex)]; if (!item?.copy_text) return; await navigator.clipboard.writeText(item.copy_text); qs('#my-work-action-status').textContent = 'Queued content copied without sending it.'; - }); + }; }); list.querySelectorAll('.draft-discard').forEach(button => { button.addEventListener('click', async () => { diff --git a/frontend/mobile-delivery-recovery.js b/frontend/mobile-delivery-recovery.js index 4a29466..d5cd470 100644 --- a/frontend/mobile-delivery-recovery.js +++ b/frontend/mobile-delivery-recovery.js @@ -109,14 +109,15 @@ } async function attend(item) { - if (typeof document === 'undefined') return false; const index = options.getIndex ? options.getIndex(item) : -1; const selector = item.quarantined ? '.draft-copy' : item.checklist_conflict ? '.draft-review-checklist' : '.draft-resume'; const action = document.querySelector('#my-work-list ' + selector + '[data-draft-index="' + index + '"]'); - if (!action) throw new Error('This delivery changed. Reopen recovery to load its current action.'); - action.click(); - return true; + if (!action) throw new Error('Delivery changed.'); + elements.dialog.close(); + action.scrollIntoView(); + action.focus(); + return await action.onclick(); } function activate() { @@ -138,7 +139,8 @@ } const result = render(); if (failure && elements?.status) { - elements.status.textContent = failure + ' Try again when the connection is ready.'; + if (active && !elements.dialog.open) elements.dialog.showModal(); + elements.status.textContent = failure + ' Try again.'; } return result; })().finally(() => { inFlight = null; }); diff --git a/tests/e2e/test_mobile_home_bootstrap_release.py b/tests/e2e/test_mobile_home_bootstrap_release.py index 5c2fb62..b7ee753 100644 --- a/tests/e2e/test_mobile_home_bootstrap_release.py +++ b/tests/e2e/test_mobile_home_bootstrap_release.py @@ -194,6 +194,40 @@ def test_release_artifact_keeps_mobile_delivery_recovery_single_flight(tmp_path: page.evaluate("Promise.all([window.__firstRecovery, window.__secondRecovery])") expect(action).to_be_enabled() assert page.evaluate("window.__deliveryAttempts") == 1 + + page.evaluate( + """ + () => { + const destination = document.createElement('button'); + destination.type = 'button'; + destination.className = 'draft-resume'; + destination.dataset.draftIndex = '0'; + destination.textContent = 'Open current review'; + window.__attentionAttempts = 0; + window.__attentionSettled = new Promise(resolve => { window.__finishAttention = resolve; }); + destination.onclick = async () => { + window.__attentionAttempts += 1; + await window.__attentionSettled; + return true; + }; + document.querySelector('#my-work-list').append(destination); + window.__attentionRecovery = createMobileDeliveryRecovery({ + getItems: () => [{outbox_id:'review-attention', status:'attention', title:'Review feedback'}], + getIndex: () => 0, + }); + window.__attentionRecovery.open(); + window.__firstAttention = window.__attentionRecovery.activate(); + window.__secondAttention = window.__attentionRecovery.activate(); + } + """ + ) + expect(page.locator("#mobile-delivery-recovery")).to_be_hidden() + expect(page.locator("#my-work-list .draft-resume")).to_be_focused() + assert page.evaluate("window.__attentionAttempts") == 1 + assert page.evaluate("document.documentElement.scrollWidth <= window.innerWidth") + page.evaluate("window.__finishAttention()") + page.evaluate("Promise.all([window.__firstAttention, window.__secondAttention])") + assert page.evaluate("window.__attentionAttempts") == 1 assert browser_errors == [] browser.close() finally: diff --git a/tests/test_frontend_bundle.py b/tests/test_frontend_bundle.py index 089a862..7bf8054 100644 --- a/tests/test_frontend_bundle.py +++ b/tests/test_frontend_bundle.py @@ -72,6 +72,8 @@ def test_product_workflows_are_stable_lazy_feature_chunks(tmp_path): assert b"function createQueueToday" in first.feature_bundles["today-timer"].runtime_bytes assert b"function createConversationPager" in first.feature_bundles["today-timer"].runtime_bytes assert b"function createConversationPager" not in first.feature_bundles["comment-actions"].runtime_bytes + assert b"function createMobileDeliveryRecovery" not in first.runtime_bytes + assert b"function createMobileDeliveryRecovery" in first.feature_bundles["today-timer"].runtime_bytes assert b"gitea_time_logged" not in first.runtime_bytes assert b"gitea_time_logged" in security_center.runtime_bytes # Core mobile workflows stay below 99 KiB, including synced Search views and Update decisions. diff --git a/tests/test_mobile_delivery_recovery.py b/tests/test_mobile_delivery_recovery.py index 056b479..ed801a0 100644 --- a/tests/test_mobile_delivery_recovery.py +++ b/tests/test_mobile_delivery_recovery.py @@ -135,6 +135,109 @@ const createRecovery = require({json.dumps(str(RECOVERY))}); } +def test_attention_handoff_closes_modal_focuses_destination_and_awaits_work(): + script = f""" +const createRecovery = require({json.dumps(str(RECOVERY))}); +(async () => {{ + let finish; + let handoffs = 0; + const settled = new Promise(resolve => {{finish = resolve}}); + const destination = {{ + focused:false, scrolled:false, + focus(){{this.focused=true}}, scrollIntoView(){{this.scrolled=true}}, + async onclick(){{handoffs += 1; await settled; return true}}, + }}; + global.document = {{ + getElementById: () => null, + querySelector: selector => selector.includes('draft-resume') ? destination : null, + }}; + const element = () => ({{textContent:'', disabled:false}}); + const elements = {{ + dialog: {{open:false, showModal(){{this.open=true}}, close(){{this.open=false}}}}, + title:element(), destination:element(), reason:element(), progress:element(), + action:element(), status:element(), + }}; + const recovery = createRecovery({{ + getItems: () => [{{outbox_id:'review', status:'attention', title:'Review feedback'}}], + getIndex: () => 3, + elements, + }}); + recovery.open(); + const first = recovery.activate(); + const second = recovery.activate(); + await Promise.resolve(); + const pending = {{ + dialogOpen:elements.dialog.open, disabled:elements.action.disabled, + focused:destination.focused, scrolled:destination.scrolled, handoffs, + }}; + finish(); + await Promise.all([first, second]); + process.stdout.write(JSON.stringify({{pending, handoffs, dialogOpen:elements.dialog.open}})); +}})(); +""" + + assert run_node(script) == { + "pending": { + "dialogOpen": False, + "disabled": True, + "focused": True, + "scrolled": True, + "handoffs": 1, + }, + "handoffs": 1, + "dialogOpen": False, + } + + +def test_failed_attention_handoff_reopens_the_same_retryable_item(): + script = f""" +const createRecovery = require({json.dumps(str(RECOVERY))}); +(async () => {{ + const destination = {{ + focus(){{}}, scrollIntoView(){{}}, + async onclick(){{throw new Error('Latest checklist unavailable')}}, + }}; + global.document = {{ + getElementById: () => null, + querySelector: () => destination, + }}; + const element = () => ({{textContent:'', disabled:false}}); + const elements = {{ + dialog: {{open:false, showModal(){{this.open=true}}, close(){{this.open=false}}}}, + title:element(), destination:element(), reason:element(), progress:element(), + action:element(), status:element(), + }}; + const recovery = createRecovery({{ + getItems: () => [{{outbox_id:'conflict', checklist_conflict:true, title:'Checklist conflict'}}], + getIndex: () => 0, + elements, + }}); + recovery.open(); + const result = await recovery.activate(); + process.stdout.write(JSON.stringify({{ + result, dialogOpen:elements.dialog.open, status:elements.status.textContent, + action:elements.action.textContent, + }})); +}})(); +""" + + assert run_node(script) == { + "result": { + "count": 1, + "current": { + "id": "conflict", + "state": "attention", + "primary": "Review changes", + "position": 1, + "total": 1, + }, + }, + "dialogOpen": True, + "status": "Latest checklist unavailable Try again.", + "action": "Review changes", + } + + def test_recovery_retains_failed_delivery_with_actionable_status(): script = f""" const createRecovery = require({json.dumps(str(RECOVERY))}); @@ -170,7 +273,7 @@ const createRecovery = require({json.dumps(str(RECOVERY))}); }, }, "disabled": False, - "status": "Network unavailable Try again when the connection is ready.", + "status": "Network unavailable Try again.", } @@ -235,6 +338,7 @@ async def test_dashboard_packages_phone_safe_guided_delivery_recovery(): assert '' in html assert "const mobileDeliveryRecovery = createMobileDeliveryRecovery({" in html assert "activate: recoverMobileDelivery," in html + assert "button.onclick=async()=>" in html assert "await (authored ? authoredOutbox : issueOutbox).retry(item.outbox_id, activeFlushLogin)" in html assert "(authored ? applyAuthoredOutboxResult : applyOutboxResult)(result)" in html assert "openDelivery: () => mobileDeliveryRecovery.open()" in html -- 2.43.0