Load comment actions only when a conversation opens #840

Merged
timmy merged 1 commits from timmy/839-lazy-comment-actions into main 2026-08-14 17:03:10 +00:00
7 changed files with 230 additions and 21 deletions

View File

@ -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;

View File

@ -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; }

View File

@ -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 '<div class="issue-comment" data-comment-id="' + Number(comment.id || 0) + '"><div class="small">' +
escapeHtml(comment.author || 'Unknown author') +
(comment.created_at ? ' · ' + escapeHtml(fmt(comment.created_at)) : '') +
@ -3109,19 +3111,19 @@
renderMarkdown(comment.body || 'No comment body provided.') + '</div></div>';
}
function renderIssueConversation(state) {
function paintIssueConversation(state, controller) {
const comments = state?.comments || [];
qs('#issue-comments').innerHTML = comments.length ?
comments.map(renderIssueComment).join('') : '<div class="muted">No comments yet.</div>';
comments.map(comment => renderIssueComment(comment, controller)).join('') : '<div class="muted">No comments yet.</div>';
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('') : '<div class="muted">No comments yet.</div>';
comments.map(comment => renderIssueComment(comment, controller)).join('') : '<div class="muted">No comments yet.</div>';
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 =>
'<div class="pull-comment-card">' + renderIssueComment(comment) + '</div>'
'<div class="pull-comment-card">' + renderIssueComment(comment, controller) + '</div>'
).join('') : '<div class="muted">No comments yet.</div>';
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');

View File

@ -595,6 +595,7 @@
<div id="issue-comments"></div>
<button class="conversation-more" id="load-older-issue-comments" type="button" hidden>Load older messages</button>
<div id="issue-conversation-status" class="small" aria-live="assertive"></div>
<button class="conversation-actions-retry" id="retry-issue-comment-actions" type="button" hidden>Retry comment tools</button>
<section class="issue-comment-composer" aria-labelledby="issue-comment-title">
<h2 id="issue-comment-title">Add comment</h2>
<textarea id="issue-comment" maxlength="10000" placeholder="Write a comment"></textarea>
@ -921,6 +922,7 @@
<button class="conversation-more" id="load-older-update-comments" type="button" hidden>Load older messages</button>
<div id="update-conversation-status" class="small" aria-live="assertive"></div>
<button class="update-retry" id="retry-update-conversation" type="button" hidden>Retry conversation</button>
<button class="conversation-actions-retry" id="retry-update-comment-actions" type="button" hidden>Retry comment tools</button>
<details>
<summary><h2>Subject context</h2></summary>
<div class="update-sheet-content muted markdown-content" id="update-subject-body"></div>
@ -995,6 +997,7 @@
<h2>Full conversation</h2><div id="pull-comments"></div>
<button class="conversation-more" id="load-older-pull-comments" type="button" hidden>Load older messages</button>
<div id="pull-conversation-status" class="small" aria-live="assertive"></div>
<button class="conversation-actions-retry" id="retry-pull-comment-actions" type="button" hidden>Retry comment tools</button>
<section class="pull-comment-composer" aria-labelledby="pull-comment-title">
<h2 id="pull-comment-title">Add comment</h2>
<textarea id="pull-comment" maxlength="10000" placeholder="Write a comment"></textarea>
@ -1182,6 +1185,7 @@
<script src="static/session.js"></script>
<script src="static/feature-loader.js"></script>
<script src="static/conversation-action-hydrator.js"></script>
<script src="static/security-center.js"></script>
<script src="static/markdown.js"></script>
<script src="static/commands.js"></script>

View File

@ -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',

View File

@ -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 '<script src="static/conversation-action-hydrator.js"></script>' 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

View File

@ -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",