Load comment actions only when a conversation opens #840
49
frontend/conversation-action-hydrator.js
Normal file
49
frontend/conversation-action-hydrator.js
Normal 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;
|
||||
|
|
@ -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; }
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
|
|
|
|||
|
|
@ -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>
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
|
|
|
|||
120
tests/test_conversation_action_hydrator.py
Normal file
120
tests/test_conversation_action_hydrator.py
Normal 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
|
||||
|
|
@ -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",
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user