feat: act on Search and Following comments (Closes #1388)
This commit is contained in:
parent
0682bbcc2c
commit
7662cca9e6
|
|
@ -81,4 +81,37 @@ function createConversationPager({ loadPage }) {
|
|||
};
|
||||
}
|
||||
|
||||
function renderConversationComment(comment, controller, escapeHtml, formatTime, renderMarkdown) {
|
||||
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(formatTime(comment.created_at)) : '') +
|
||||
'</div>' + actions + '<div class="issue-sheet-content markdown-content">' +
|
||||
renderMarkdown(comment.body || 'No comment body provided.') + '</div></div>';
|
||||
}
|
||||
|
||||
function createConversationRenderers({qs,renderComment,updateReadPosition,getSelectedUpdate}) {
|
||||
function status(kind, comments, total) {
|
||||
qs('#' + kind + '-conversation-status').textContent = comments.length ?
|
||||
comments.length + ' of ' + Math.max(total || 0, comments.length) + ' messages loaded.' : 'No comments yet.';
|
||||
}
|
||||
function paint(kind, state, controller) {
|
||||
const comments = state?.comments || [];
|
||||
qs('#' + kind + '-comments').innerHTML = comments.length ? comments.map(comment =>
|
||||
kind === 'pull' ? '<div class="pull-comment-card">' + renderComment(comment,controller) + '</div>' :
|
||||
renderComment(comment,controller)).join('') : '<div class="muted">No comments yet.</div>';
|
||||
qs('#load-older-' + kind + '-comments').hidden = !Number.isInteger(state?.older_page);
|
||||
status(kind,comments,state?.total);
|
||||
if (kind === 'update') {
|
||||
const newest = qs('#update-comments .issue-comment:last-child');
|
||||
updateReadPosition.ready(String(getSelectedUpdate()?.notification_id || ''),newest);
|
||||
}
|
||||
}
|
||||
return {
|
||||
paintIssueConversation:(state,controller)=>paint('issue',state,controller),
|
||||
paintPullConversation:(state,controller)=>paint('pull',state,controller),
|
||||
paintUpdateConversation:(state,controller)=>paint('update',state,controller),
|
||||
};
|
||||
}
|
||||
|
||||
if (typeof module !== 'undefined' && module.exports) module.exports = createConversationPager;
|
||||
|
|
|
|||
|
|
@ -4097,46 +4097,17 @@
|
|||
toggle?.focus();
|
||||
}
|
||||
|
||||
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)) : '') +
|
||||
'</div>' + actions + '<div class="issue-sheet-content markdown-content">' +
|
||||
renderMarkdown(comment.body || 'No comment body provided.') + '</div></div>';
|
||||
}
|
||||
|
||||
function paintIssueConversation(state, controller) {
|
||||
const comments = state?.comments || [];
|
||||
qs('#issue-comments').innerHTML = comments.length ?
|
||||
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 paintUpdateConversation(state, controller) {
|
||||
const comments = state?.comments || [];
|
||||
qs('#update-comments').innerHTML = comments.length ?
|
||||
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.';
|
||||
const newest = qs('#update-comments .issue-comment:last-child');
|
||||
updateReadPosition.ready(String(selectedUpdate?.notification_id || ''), newest);
|
||||
}
|
||||
|
||||
function paintPullConversation(state, controller) {
|
||||
const comments = state?.comments || [];
|
||||
qs('#pull-comments').innerHTML = comments.length ? comments.map(comment =>
|
||||
'<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 ?
|
||||
comments.length + ' of ' + Math.max(state.total || 0, comments.length) + ' messages loaded.' : 'No comments yet.';
|
||||
}
|
||||
const {paintIssueConversation,paintPullConversation,paintUpdateConversation}=createConversationRenderers({
|
||||
qs,renderComment:(comment,controller)=>renderConversationComment(
|
||||
comment,controller,escapeHtml,fmt,renderMarkdown),
|
||||
updateReadPosition,getSelectedUpdate:()=>selectedUpdate,
|
||||
});
|
||||
|
||||
function commentSurface(selector) {
|
||||
if (selector === '#search-preview-comments') return {
|
||||
context:{kind:searchPreviewDetail.kind,item:searchPreviewDetail}, pager:searchPreview.commentPager(),
|
||||
render:state=>showSearchConversationWithActions(state), status:qs('#search-preview-conversation-status'),
|
||||
};
|
||||
if (selector === '#issue-comments') return {
|
||||
context:{kind:'issue',item:selectedIssue}, pager:issueConversation,
|
||||
render:renderIssueConversation, status:qs('#issue-sheet-status'),
|
||||
|
|
@ -5664,8 +5635,14 @@
|
|||
commandSearch.setScope(scope);
|
||||
}
|
||||
let searchPreviewDetail = null;
|
||||
const renderSearchConversation = conversation =>
|
||||
renderSearchPreviewConversation(conversation, document, escapeHtml, fmt, renderMarkdown);
|
||||
const showSearchConversationWithActions = createSearchPreviewConversationActions({
|
||||
hydrator:actionHydrator, rootNode:qs('#search-preview-comments'),
|
||||
retry:qs('#retry-search-preview-comment-actions'),
|
||||
paint:(conversation,controller)=>renderSearchPreviewConversation(
|
||||
conversation,document,escapeHtml,fmt,renderMarkdown,controller),
|
||||
wire:controller=>wireCommentActions('#search-preview-comments',controller),
|
||||
});
|
||||
const renderSearchConversation = conversation => { void showSearchConversationWithActions(conversation); };
|
||||
function renderSearchPreview(state) {
|
||||
followingQueue.preview(state);
|
||||
const sheet = qs('#search-preview');
|
||||
|
|
|
|||
|
|
@ -901,6 +901,7 @@
|
|||
<div id="search-preview-comments"></div>
|
||||
<div id="search-preview-conversation-status" class="small" aria-live="polite"></div>
|
||||
<button id="retry-search-preview-conversation" type="button" hidden>Retry conversation</button>
|
||||
<button class="conversation-actions-retry" id="retry-search-preview-comment-actions" type="button" hidden>Retry comment tools</button>
|
||||
<button id="load-older-search-preview-comments" type="button" hidden>Load older messages</button>
|
||||
</section>
|
||||
<section id="search-preview-review" class="search-preview-review" aria-labelledby="search-preview-review-title" hidden>
|
||||
|
|
|
|||
|
|
@ -148,7 +148,7 @@
|
|||
body:JSON.stringify({body}),
|
||||
}),
|
||||
});
|
||||
root.renderSearchPreviewConversation = (conversation, document, escapeHtml, formatTime, renderMarkdown) => {
|
||||
root.renderSearchPreviewConversation = (conversation, document, escapeHtml, formatTime, renderMarkdown, actions) => {
|
||||
const comments = document.querySelector('#search-preview-comments');
|
||||
const status = document.querySelector('#search-preview-conversation-status');
|
||||
const retry = document.querySelector('#retry-search-preview-conversation');
|
||||
|
|
@ -162,11 +162,12 @@
|
|||
new Date(comment.created_at).getTime() > new Date(conversation.reviewedAt).getTime());
|
||||
const newCount = (conversation.comments || []).filter(isNew).length;
|
||||
comments.innerHTML = (conversation.comments || []).map(comment =>
|
||||
'<article class="search-preview-comment' + (isNew(comment) ? ' new-since-review' : '') +
|
||||
'<article class="search-preview-comment issue-comment' + (isNew(comment) ? ' new-since-review' : '') +
|
||||
'" data-comment-id="' + Number(comment.id || 0) + '">' +
|
||||
'<div class="small">' + escapeHtml(comment.author || 'Unknown author') +
|
||||
(comment.created_at ? ' · ' + escapeHtml(formatTime(comment.created_at)) : '') +
|
||||
(isNew(comment) ? ' · <strong>New since last review</strong>' : '') + '</div>' +
|
||||
(actions?.actionHtml?.(comment) || '') +
|
||||
'<div class="markdown-content">' + renderMarkdown(comment.body || '') + '</div></article>'
|
||||
).join('');
|
||||
retry.hidden = conversation.status !== 'error';
|
||||
|
|
@ -181,6 +182,19 @@
|
|||
(conversation.comments.length === 1 ? ' message' : ' messages') +
|
||||
(newCount ? ' · ' + newCount + ' new since last review' : '') + '.';
|
||||
};
|
||||
root.createSearchPreviewConversationActions = ({hydrator,rootNode,retry,paint,wire}) => {
|
||||
let latest = null;
|
||||
const show = conversation => {
|
||||
latest = conversation;
|
||||
if (!conversation) {
|
||||
paint(null, null);
|
||||
return Promise.resolve(false);
|
||||
}
|
||||
return hydrator.show({root:rootNode,state:conversation,paint,retry,wire});
|
||||
};
|
||||
retry.addEventListener('click', () => { if (latest) void show(latest); });
|
||||
return show;
|
||||
};
|
||||
root.renderSearchPreviewReply = (state, detail, preview, document) => {
|
||||
const section = document.querySelector('.search-preview-reply');
|
||||
const input = document.querySelector('#search-preview-reply');
|
||||
|
|
@ -417,7 +431,27 @@
|
|||
}
|
||||
}
|
||||
|
||||
const conversationPager = {
|
||||
snapshot() {
|
||||
return {...(conversation || {}), comments:(conversation?.comments || []).map(comment => ({...comment}))};
|
||||
},
|
||||
replace(comment) {
|
||||
if (!comment || !Number.isInteger(comment.id) || !conversation) return conversationPager.snapshot();
|
||||
conversation = {...conversation, comments:(conversation.comments || []).map(existing =>
|
||||
existing.id === comment.id ? {...comment} : existing)};
|
||||
publish({status:'ready', item:current, detail:current, conversation});
|
||||
return conversationPager.snapshot();
|
||||
},
|
||||
remove(commentId) {
|
||||
if (!conversation) return conversationPager.snapshot();
|
||||
conversation = {...conversation, comments:(conversation.comments || []).filter(comment => comment.id !== commentId)};
|
||||
publish({status:'ready', item:current, detail:current, conversation});
|
||||
return conversationPager.snapshot();
|
||||
},
|
||||
};
|
||||
|
||||
const api = {
|
||||
commentPager() { return conversationPager; },
|
||||
hasReplyAttachments() {
|
||||
return Boolean(hasAttachments?.());
|
||||
},
|
||||
|
|
|
|||
|
|
@ -35,12 +35,12 @@ FEATURE_SOURCES = {
|
|||
"security-center": ("static/security-center.js",),
|
||||
"planning": (
|
||||
"static/plan-today.js", "static/plan-today-readiness.js",
|
||||
"static/plan-today-preview.js", "static/today-rollover.js", "static/today-readiness.js", "static/following.js",
|
||||
"static/plan-today-preview.js", "static/today-rollover.js", "static/today-readiness.js", "static/following.js", "static/search-preview.js",
|
||||
"static/tomorrow-plan.js", "static/week-calendar.js", "static/week-calendar-import.js", "static/week-plan.js", "static/today-week-reschedule.js", "static/search-week-plan.js", "static/search-batch-plan.js", "static/agenda-session-launcher.js", "static/mobile-plan-today-nav.js",
|
||||
),
|
||||
"today-timer": (
|
||||
"static/mobile-app-badge.js", "static/conversation.js", "static/widgets.js", "static/voice-transcript-store.js", "static/voice-conversation-capture.js", "static/mobile-launch.js", "static/mobile-insights.js", "static/mobile-app-shortcuts.js", "static/mobile-find-work-nav.js", "static/mobile-pull-refresh.js", "static/live-data-status.js", "static/mobile-composer-viewport.js",
|
||||
"static/today-completion.js", "static/card-planning.js", "static/work-detail-position.js", "static/work-route.js", "static/commands.js", "static/saved-searches.js", "static/task-overlay-history.js", "static/search-preview.js", "static/mobile-search-preview-nav.js", "static/search-reply-draft-store.js", "static/conversation-reply-draft-store.js", "static/conversation-photo-drafts.js", "static/search-defer.js", "static/mobile-search-viewport.js", "static/agenda-replan.js", "static/agenda-calendar.js", "static/my-work.js", "static/protect-today.js", "static/mobile-today-command-bar.js", "static/mobile-task-dock.js", "static/mobile-first-task.js", "static/mobile-work-entry.js", "static/mobile-queue-launcher.js", "static/mobile-delivery-recovery.js", "static/mobile-start-day.js", "static/update-triage-session.js", "static/update-review-handoff.js", "static/update-triage-launcher.js", "static/update-triage-gesture.js", "static/notification-undo.js", "static/today-timer.js", "static/today-break.js", "static/today-progress.js", "static/today-lock-screen.js", "static/today-session-sync.js", "static/today-recap.js", "static/today-wrap-up.js", "static/today-summary.js", "static/today-handoff.js",
|
||||
"static/today-completion.js", "static/card-planning.js", "static/work-detail-position.js", "static/work-route.js", "static/commands.js", "static/saved-searches.js", "static/task-overlay-history.js", "static/mobile-search-preview-nav.js", "static/search-reply-draft-store.js", "static/conversation-reply-draft-store.js", "static/conversation-photo-drafts.js", "static/search-defer.js", "static/mobile-search-viewport.js", "static/agenda-replan.js", "static/agenda-calendar.js", "static/my-work.js", "static/protect-today.js", "static/mobile-today-command-bar.js", "static/mobile-task-dock.js", "static/mobile-first-task.js", "static/mobile-work-entry.js", "static/mobile-queue-launcher.js", "static/mobile-delivery-recovery.js", "static/mobile-start-day.js", "static/update-triage-session.js", "static/update-review-handoff.js", "static/update-triage-launcher.js", "static/update-triage-gesture.js", "static/notification-undo.js", "static/today-timer.js", "static/today-break.js", "static/today-progress.js", "static/today-lock-screen.js", "static/today-session-sync.js", "static/today-recap.js", "static/today-wrap-up.js", "static/today-summary.js", "static/today-handoff.js",
|
||||
"static/later-work.js", "static/detail-defer.js", "static/later-picker.js", "static/drafts.js", "static/unfiled-captures.js", "static/unfiled-draft-sync.js",
|
||||
"static/assign-and-start.js", "static/filed-claim.js", "static/queue-today.js", "static/create-and-start.js",
|
||||
"static/draft-filing-session.js", "static/draft-capacity-dialog.js", "static/work-selection.js",
|
||||
|
|
|
|||
|
|
@ -75,3 +75,41 @@ def test_comment_reactions_are_touch_safe_and_focus_preserving_at_320px():
|
|||
["api/v1/repos/stackchain/api/issues/7/comments/91/reactions/heart", "PUT"],
|
||||
]
|
||||
browser.close()
|
||||
|
||||
|
||||
def test_search_following_conversation_actions_are_touch_safe_at_320px():
|
||||
with sync_playwright() as playwright:
|
||||
browser = playwright.chromium.launch(headless=True)
|
||||
page = browser.new_page(viewport={"width": 320, "height": 568})
|
||||
page.set_content(
|
||||
'<main><section id="search-preview-conversation" class="search-preview-conversation">'
|
||||
'<div id="search-preview-comments"></div>'
|
||||
'<div id="search-preview-conversation-status"></div>'
|
||||
'<button id="retry-search-preview-conversation" hidden></button>'
|
||||
'<button id="load-older-search-preview-comments" hidden></button>'
|
||||
'</section></main>'
|
||||
)
|
||||
page.add_style_tag(path=FRONTEND / "dashboard.css")
|
||||
page.add_script_tag(path=FRONTEND / "search-preview.js")
|
||||
page.add_script_tag(path=FRONTEND / "comment-actions.js")
|
||||
page.evaluate(
|
||||
"""
|
||||
const actions=createCommentActions({fetchJson:async()=>({}),getLogin:()=> 'timmy'});
|
||||
renderSearchPreviewConversation({status:'ready',comments:[
|
||||
{id:41,author:'timmy',body:'Correct this from Following'},
|
||||
{id:42,author:'alexander',body:'React without leaving Search'},
|
||||
]},document,value=>String(value),value=>String(value),value=>String(value),actions);
|
||||
"""
|
||||
)
|
||||
|
||||
expect(page.locator(".search-preview-comment.issue-comment")).to_have_count(2)
|
||||
expect(page.get_by_role("button", name="Edit")).to_have_count(1)
|
||||
expect(page.get_by_role("button", name="Delete")).to_have_count(1)
|
||||
expect(page.get_by_role("button", name="React to this comment")).to_have_count(2)
|
||||
for control in page.locator("[data-comment-action], [data-comment-reactions-open]").all():
|
||||
box = control.bounding_box()
|
||||
assert box is not None and box["height"] >= 44
|
||||
assert page.evaluate(
|
||||
"document.documentElement.scrollWidth > document.documentElement.clientWidth"
|
||||
) is False
|
||||
browser.close()
|
||||
|
|
|
|||
|
|
@ -133,3 +133,75 @@ def test_comment_action_retry_is_a_phone_sized_inline_control():
|
|||
rule = css.split(".conversation-actions-retry", 1)[1].split("}", 1)[0]
|
||||
assert "min-height:44px" in rule
|
||||
assert "max-width:100%" in rule
|
||||
|
||||
|
||||
def test_search_and_following_preview_hydrates_owned_comment_actions_lazily():
|
||||
preview = FRONTEND / "search-preview.js"
|
||||
script = f"""
|
||||
require({json.dumps(str(preview))});
|
||||
const nodes = {{
|
||||
'#search-preview-comments': {{innerHTML:'',textContent:''}},
|
||||
'#search-preview-conversation-status': {{textContent:''}},
|
||||
'#retry-search-preview-conversation': {{hidden:true}},
|
||||
'#load-older-search-preview-comments': {{hidden:true,disabled:false}},
|
||||
}};
|
||||
const document = {{querySelector: selector => nodes[selector]}};
|
||||
const actions = {{actionHtml: comment => comment.author === 'timmy'
|
||||
? '<button data-comment-action="edit">Edit</button><button data-comment-reactions-open>React</button>'
|
||||
: '<button data-comment-reactions-open>React</button>'}};
|
||||
renderSearchPreviewConversation({{
|
||||
status:'ready', reviewedAt:'2026-08-25T08:00:00Z',
|
||||
comments:[
|
||||
{{id:41,author:'timmy',body:'Mine',created_at:'2026-08-25T09:00:00Z'}},
|
||||
{{id:42,author:'alexander',body:'Theirs',created_at:'2026-08-25T07:00:00Z'}},
|
||||
],
|
||||
}}, document, String, String, String, actions);
|
||||
process.stdout.write(JSON.stringify({{html:nodes['#search-preview-comments'].innerHTML}}));
|
||||
"""
|
||||
output = run_node(script)["html"]
|
||||
|
||||
assert output.count('class="search-preview-comment issue-comment') == 2
|
||||
assert output.count("data-comment-reactions-open") == 2
|
||||
assert output.count('data-comment-action="edit"') == 1
|
||||
assert 'data-comment-id="41"' in output
|
||||
assert "new-since-review" in output
|
||||
|
||||
html = (FRONTEND / "index.html").read_text()
|
||||
javascript = (FRONTEND / "dashboard.js").read_text()
|
||||
assert 'id="retry-search-preview-comment-actions"' in html
|
||||
assert "showSearchConversationWithActions" in javascript
|
||||
assert "context:{kind:searchPreviewDetail.kind,item:searchPreviewDetail}" in javascript
|
||||
|
||||
|
||||
def test_search_preview_comment_pager_updates_exact_visible_comment_in_place():
|
||||
preview = FRONTEND / "search-preview.js"
|
||||
script = f"""
|
||||
const createSearchPreview = require({json.dumps(str(preview))});
|
||||
(async () => {{
|
||||
const states=[];
|
||||
const controller=createSearchPreview({{
|
||||
fetchJson:async item=>({{...item,title:'Visible'}}),
|
||||
fetchConversation:async()=>({{comments:[
|
||||
{{id:41,author:'timmy',body:'Old'}},{{id:42,author:'alexander',body:'Keep'}},
|
||||
],older_page:null}}),
|
||||
mutate:async()=>({{}}), onState:state=>states.push(state),
|
||||
}});
|
||||
await controller.open({{repository:'stackchain/api',number:9,kind:'pull'}});
|
||||
await new Promise(resolve=>setTimeout(resolve,0));
|
||||
const pager=controller.commentPager();
|
||||
pager.replace({{id:41,author:'timmy',body:'Corrected'}});
|
||||
const replaced=pager.snapshot().comments;
|
||||
pager.remove(41);
|
||||
process.stdout.write(JSON.stringify({{
|
||||
replaced:replaced.map(comment=>[comment.id,comment.body]),
|
||||
remaining:pager.snapshot().comments.map(comment=>comment.id),
|
||||
published:states.at(-1).conversation.comments.map(comment=>comment.id),
|
||||
}}));
|
||||
}})().catch(error=>{{console.error(error);process.exit(1);}});
|
||||
"""
|
||||
|
||||
assert run_node(script) == {
|
||||
"replaced": [[41, "Corrected"], [42, "Keep"]],
|
||||
"remaining": [42],
|
||||
"published": [42],
|
||||
}
|
||||
|
|
|
|||
|
|
@ -221,7 +221,7 @@ def test_markdown_renderer_allows_only_safe_links_and_keeps_html_inert():
|
|||
|
||||
|
||||
def test_all_read_only_work_bodies_use_the_shared_markdown_renderer():
|
||||
dashboard = (FRONTEND / "dashboard.js").read_text()
|
||||
dashboard = (FRONTEND / "dashboard.js").read_text() + (FRONTEND / "conversation.js").read_text()
|
||||
|
||||
expected_paths = (
|
||||
"renderMarkdown(comment.body || 'No comment body provided.')",
|
||||
|
|
|
|||
|
|
@ -82,6 +82,7 @@ async def test_dashboard_mounts_mobile_new_activity_positioning_flow():
|
|||
assert '<script src="static/update-read-position.js"></script>' in html
|
||||
assert 'id="jump-update-new-activity"' in html
|
||||
assert "updateReadPosition.open(String(item.notification_id))" in html
|
||||
assert "updateReadPosition.ready(String(selectedUpdate?.notification_id || ''), newest)" in html
|
||||
source = html + (POSITIONER.parent / "conversation.js").read_text()
|
||||
assert "updateReadPosition.ready(String(getSelectedUpdate()?.notification_id || ''),newest)" in source
|
||||
assert ".update-new-activity" in html
|
||||
assert "BASE + 'static/update-read-position.js'" in (POSITIONER.parent / "service-worker.js").read_text()
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user