From f094438dabddf6badbe7a9aff52f4b4643588c33 Mon Sep 17 00:00:00 2001 From: timmy Date: Fri, 14 Aug 2026 17:00:31 +0000 Subject: [PATCH] Load comment actions on conversation demand --- frontend/conversation-action-hydrator.js | 49 +++++++++ frontend/dashboard.css | 1 + frontend/dashboard.js | 75 +++++++++---- frontend/index.html | 4 + frontend/service-worker.js | 1 + tests/test_conversation_action_hydrator.py | 120 +++++++++++++++++++++ tests/test_service_worker.py | 1 + 7 files changed, 230 insertions(+), 21 deletions(-) create mode 100644 frontend/conversation-action-hydrator.js create mode 100644 tests/test_conversation_action_hydrator.py diff --git a/frontend/conversation-action-hydrator.js b/frontend/conversation-action-hydrator.js new file mode 100644 index 0000000..1b22d6e --- /dev/null +++ b/frontend/conversation-action-hydrator.js @@ -0,0 +1,49 @@ +function createConversationActionHydrator({ load, activate }) { + let actions = null; + let pending = null; + const wiredRoots = new WeakSet(); + + function ensure() { + if (actions) return Promise.resolve(actions); + if (!pending) { + pending = load().then(() => { + actions = activate(); + return actions; + }).catch(error => { + pending = null; + throw error; + }); + } + return pending; + } + + function show({ root, state, paint, wire, retry }) { + if (actions) { + retry.hidden = true; + if (!wiredRoots.has(root)) { + wire(actions); + wiredRoots.add(root); + } + paint(state, actions); + return Promise.resolve(true); + } + + paint(state, null); + retry.hidden = true; + return ensure().then(controller => { + if (!wiredRoots.has(root)) { + wire(controller); + wiredRoots.add(root); + } + paint(state, controller); + return true; + }).catch(() => { + retry.hidden = false; + return false; + }); + } + + return { show, ready: () => Boolean(actions) }; +} + +if (typeof module !== 'undefined' && module.exports) module.exports = createConversationActionHydrator; diff --git a/frontend/dashboard.css b/frontend/dashboard.css index 61d669e..b06d92a 100644 --- a/frontend/dashboard.css +++ b/frontend/dashboard.css @@ -696,6 +696,7 @@ textarea { resize: vertical; min-height: 120px; } .pull-sheet-content { overflow-wrap:anywhere; white-space:pre-wrap; } .pull-file, .pull-comment-card { margin:8px 0; padding:10px; border:1px solid #203a5c; border-radius:10px; } .conversation-more { min-height:44px; width:100%; margin:8px 0; } +.conversation-actions-retry { min-height:44px; max-width:100%; margin:8px 0; } .pull-file-toggle, .pull-review-file { min-height:44px; width:100%; } .pull-file-toggle { display:flex; justify-content:space-between; align-items:center; gap:8px; text-align:left; } .pull-review-file { margin-top:8px; } diff --git a/frontend/dashboard.js b/frontend/dashboard.js index c6e2fc9..99f9cd7 100644 --- a/frontend/dashboard.js +++ b/frontend/dashboard.js @@ -614,14 +614,16 @@ 'comment-actions': document.querySelector('meta[name="stackchain-feature-comment-actions"]')?.content || '', }, }); - await commentActionFeatures.run('comment-actions', { - status: qs('#my-work-action-status'), retryLabel:'Reload to retry comment actions.', - }, () => { - commentActions = createCommentActions({ - fetchJson: fetchReviewJson, - getLogin: () => confirmedOwnerLogin, - confirmDelete: message => window.confirm(message), - }); + const commentActionHydrator = createConversationActionHydrator({ + load: () => commentActionFeatures.load('comment-actions'), + activate: () => { + commentActions = createCommentActions({ + fetchJson: fetchReviewJson, + getLogin: () => confirmedOwnerLogin, + confirmDelete: message => window.confirm(message), + }); + return commentActions; + }, }); const issueCaptureFeatures = createFeatureLoader({ document, @@ -3100,8 +3102,8 @@ toggle?.focus(); } - function renderIssueComment(comment) { - const actions = commentActions.actionHtml?.(comment) || ''; + function renderIssueComment(comment, controller = commentActions) { + const actions = controller?.actionHtml?.(comment) || ''; return '
' + escapeHtml(comment.author || 'Unknown author') + (comment.created_at ? ' ยท ' + escapeHtml(fmt(comment.created_at)) : '') + @@ -3109,19 +3111,19 @@ renderMarkdown(comment.body || 'No comment body provided.') + '
'; } - function renderIssueConversation(state) { + function paintIssueConversation(state, controller) { const comments = state?.comments || []; qs('#issue-comments').innerHTML = comments.length ? - comments.map(renderIssueComment).join('') : '
No comments yet.
'; + comments.map(comment => renderIssueComment(comment, controller)).join('') : '
No comments yet.
'; qs('#load-older-issue-comments').hidden = !Number.isInteger(state?.older_page); qs('#issue-conversation-status').textContent = comments.length ? comments.length + ' of ' + Math.max(state.total || 0, comments.length) + ' messages loaded.' : 'No comments yet.'; } - function renderUpdateConversation(state) { + function paintUpdateConversation(state, controller) { const comments = state?.comments || []; qs('#update-comments').innerHTML = comments.length ? - comments.map(renderIssueComment).join('') : '
No comments yet.
'; + comments.map(comment => renderIssueComment(comment, controller)).join('') : '
No comments yet.
'; qs('#load-older-update-comments').hidden = !Number.isInteger(state?.older_page); qs('#update-conversation-status').textContent = comments.length ? comments.length + ' of ' + Math.max(state.total || 0, comments.length) + ' messages loaded.' : 'No comments yet.'; @@ -3129,10 +3131,10 @@ updateReadPosition.ready(String(selectedUpdate?.notification_id || ''), newest); } - function renderPullConversation(state) { + function paintPullConversation(state, controller) { const comments = state?.comments || []; qs('#pull-comments').innerHTML = comments.length ? comments.map(comment => - '
' + renderIssueComment(comment) + '
' + '
' + renderIssueComment(comment, controller) + '
' ).join('') : '
No comments yet.
'; qs('#load-older-pull-comments').hidden = !Number.isInteger(state?.older_page); qs('#pull-conversation-status').textContent = comments.length ? @@ -3154,16 +3156,47 @@ }; } - function wireCommentActions(selector) { - commentActions.wire({ + function wireCommentActions(selector, controller = commentActions) { + controller.wire({ root:qs(selector), getSurface:()=>commentSurface(selector), isOffline:()=>offlineWorkMode || navigator.onLine === false, escapeHtml, }); } - wireCommentActions('#issue-comments'); - wireCommentActions('#pull-comments'); - wireCommentActions('#update-comments'); + const conversationActionSurfaces = { + issue: { selector:'#issue-comments', paint:paintIssueConversation }, + pull: { selector:'#pull-comments', paint:paintPullConversation }, + update: { selector:'#update-comments', paint:paintUpdateConversation }, + }; + const latestConversationStates = {}; + + function wireConversationActions(kind) { + if (kind === 'issue') wireCommentActions('#issue-comments'); + if (kind === 'pull') wireCommentActions('#pull-comments'); + if (kind === 'update') wireCommentActions('#update-comments'); + } + + function showConversationWithActions(kind, state) { + const surface = conversationActionSurfaces[kind]; + latestConversationStates[kind] = state; + return commentActionHydrator.show({ + root:qs(surface.selector), state, paint:surface.paint, + retry:qs('#retry-' + kind + '-comment-actions'), + wire:controller => wireConversationActions(kind, controller), + }); + } + + function renderIssueConversation(state) { void showConversationWithActions('issue', state); } + function renderPullConversation(state) { void showConversationWithActions('pull', state); } + function renderUpdateConversation(state) { void showConversationWithActions('update', state); } + + function retryConversationActions(kind) { + const state = latestConversationStates[kind]; + if (state) void showConversationWithActions(kind, state); + } + qs('#retry-issue-comment-actions').addEventListener('click', () => retryConversationActions('issue')); + qs('#retry-pull-comment-actions').addEventListener('click', () => retryConversationActions('pull')); + qs('#retry-update-comment-actions').addEventListener('click', () => retryConversationActions('update')); function renderIssueLabelEditor(item, confirmedNames, labels) { const list = qs('#issue-label-list'); diff --git a/frontend/index.html b/frontend/index.html index 85efcdc..c4be4d2 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -595,6 +595,7 @@
+

Add comment

@@ -921,6 +922,7 @@
+

Subject context
@@ -995,6 +997,7 @@

Full conversation

+

Add comment

@@ -1182,6 +1185,7 @@ + diff --git a/frontend/service-worker.js b/frontend/service-worker.js index 06000aa..9d01b89 100644 --- a/frontend/service-worker.js +++ b/frontend/service-worker.js @@ -14,6 +14,7 @@ const SHELL = [ BASE + 'static/icons/stackchain-512.png', BASE + 'static/session.js', BASE + 'static/feature-loader.js', + BASE + 'static/conversation-action-hydrator.js', BASE + 'static/security-center.js', BASE + 'static/markdown.js', BASE + 'static/commands.js', diff --git a/tests/test_conversation_action_hydrator.py b/tests/test_conversation_action_hydrator.py new file mode 100644 index 0000000..b63e186 --- /dev/null +++ b/tests/test_conversation_action_hydrator.py @@ -0,0 +1,120 @@ +import json +import subprocess +from pathlib import Path + + +FRONTEND = Path(__file__).parents[1] / "frontend" +HYDRATOR = FRONTEND / "conversation-action-hydrator.js" + + +def run_node(script: str): + result = subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True) + return json.loads(result.stdout) + + +def test_conversation_paints_before_optional_actions_load_and_hydrates_once(): + script = f""" +const createConversationActionHydrator = require({json.dumps(str(HYDRATOR))}); +const events = []; +let finishLoad; +const hydrator = createConversationActionHydrator({{ + load: () => {{ + events.push('load'); + return new Promise(resolve => {{ finishLoad = resolve; }}); + }}, + activate: () => ({{name:'actions'}}), +}}); +const root = {{}}; +const retry = {{hidden:false}}; +const status = {{textContent:'2 messages loaded.'}}; +const state = {{comments:[1,2]}}; +const pending = hydrator.show({{ + root, state, retry, status, + paint: (_state, actions) => events.push(actions ? 'paint-actions' : 'paint-core'), + wire: () => events.push('wire'), +}}); +events.push('returned'); +finishLoad(); +pending.then(() => {{ + hydrator.show({{ + root, state, retry, status, + paint: (_state, actions) => events.push(actions ? 'paint-actions-again' : 'paint-core-again'), + wire: () => events.push('wire-again'), + }}).then(() => process.stdout.write(JSON.stringify({{events,retry,status:status.textContent}}))); +}}); +""" + output = run_node(script) + + assert output == { + "events": [ + "paint-core", "load", "returned", "wire", "paint-actions", + "paint-actions-again", + ], + "retry": {"hidden": True}, + "status": "2 messages loaded.", + } + + +def test_failed_comment_actions_keep_conversation_usable_and_retry_in_place(): + script = f""" +const createConversationActionHydrator = require({json.dumps(str(HYDRATOR))}); +const events = []; +let attempt = 0; +const hydrator = createConversationActionHydrator({{ + load: async () => {{ + attempt += 1; + if (attempt === 1) throw new Error('chunk unavailable'); + }}, + activate: () => ({{name:'actions'}}), +}}); +const root = {{}}; +const retry = {{hidden:true}}; +const status = {{textContent:'2 messages loaded.'}}; +const options = {{ + root, retry, status, state:{{comments:[1,2]}}, + paint: (_state, actions) => events.push(actions ? 'actions' : 'core'), + wire: () => events.push('wire'), +}}; +(async () => {{ + const failed = await hydrator.show(options); + events.push('reply-still-usable'); + const recovered = await hydrator.show(options); + process.stdout.write(JSON.stringify({{ + failed, recovered, attempt, events, retry, status:status.textContent, + }})); +}})(); +""" + output = run_node(script) + + assert output == { + "failed": False, + "recovered": True, + "attempt": 2, + "events": ["core", "reply-still-usable", "core", "wire", "actions"], + "retry": {"hidden": True}, + "status": "2 messages loaded.", + } + + +def test_issue_pull_and_update_conversations_trigger_optional_actions_not_startup(): + html = (FRONTEND / "index.html").read_text() + javascript = (FRONTEND / "dashboard.js").read_text() + + assert '' in html + assert html.index("static/conversation-action-hydrator.js") < html.index("static/dashboard.js") + assert "await commentActionFeatures.run('comment-actions'" not in javascript + assert "load: () => commentActionFeatures.load('comment-actions')" in javascript + assert "createConversationActionHydrator" in javascript + for kind in ("issue", "pull", "update"): + assert f"retry-{kind}-comment-actions" in html + assert f"showConversationWithActions('{kind}'" in javascript + assert f"qs('#retry-{kind}-comment-actions').addEventListener" in javascript + + +def test_comment_action_retry_is_a_phone_sized_inline_control(): + css = (FRONTEND / "dashboard.css").read_text() + + assert ".conversation-actions-retry" in css + rule = css.split(".conversation-actions-retry", 1)[1].split("}", 1)[0] + assert "min-height:44px" in rule + assert "max-width:100%" in rule diff --git a/tests/test_service_worker.py b/tests/test_service_worker.py index fbdce1b..6c3f4f8 100644 --- a/tests/test_service_worker.py +++ b/tests/test_service_worker.py @@ -787,6 +787,7 @@ def test_install_precaches_complete_subpath_scoped_app_shell(): "/dashboard/static/icons/stackchain-512.png", "/dashboard/static/session.js", "/dashboard/static/feature-loader.js", + "/dashboard/static/conversation-action-hydrator.js", "/dashboard/static/security-center.js", "/dashboard/static/markdown.js", "/dashboard/static/commands.js",