From ccc9b928fe777e8d0bb3fe5f03a6a7aa97e5af2b Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 14 Oct 2026 20:07:31 +0800 Subject: [PATCH] refactor: issue comment context menu and buttons (#39771) 1. Support show "Quote reply" shortcut in help menu, fix the TODO 2. Help #36862 to reuse existing "issue/comment context menu" --- templates/repo/diff/comment_form.tmpl | 10 +- templates/repo/diff/comments.tmpl | 4 +- templates/repo/diff/new_review.tmpl | 17 ++- templates/repo/issue/view_content.tmpl | 2 +- .../repo/issue/view_content/comments.tmpl | 4 +- .../repo/issue/view_content/context_menu.tmpl | 57 ++++----- .../repo/issue/view_content/conversation.tmpl | 32 ++--- tests/e2e/pr-review.test.ts | 28 ++++- tests/integration/pull_diff_test.go | 9 +- web_src/css/review.css | 9 +- web_src/js/features/repo-diff.ts | 2 +- web_src/js/features/repo-issue-edit.test.ts | 16 +-- web_src/js/features/repo-issue-edit.ts | 40 +++--- web_src/js/features/repo-issue.ts | 115 ++++++++---------- web_src/js/features/repo-legacy.ts | 11 +- web_src/js/modules/shortcut.ts | 40 +++--- 16 files changed, 205 insertions(+), 191 deletions(-) diff --git a/templates/repo/diff/comment_form.tmpl b/templates/repo/diff/comment_form.tmpl index 08c3750d013..a8c209f033b 100644 --- a/templates/repo/diff/comment_form.tmpl +++ b/templates/repo/diff/comment_form.tmpl @@ -27,19 +27,19 @@
{{if $.reply}} - + {{else}} {{if $.root.CurrentReview}} - + {{else}} - - + + {{end}} {{end}} {{if or (not $.HasComments) $.hidden}} - + {{end}}
diff --git a/templates/repo/diff/comments.tmpl b/templates/repo/diff/comments.tmpl index 546a0229a78..449747f20b9 100644 --- a/templates/repo/diff/comments.tmpl +++ b/templates/repo/diff/comments.tmpl @@ -1,4 +1,4 @@ -{{range .comments}} +{{range $comment := .comments}} {{$createdStr:= DateUtils.TimeSince .CreatedUnix}}
@@ -53,7 +53,7 @@ {{if not $.root.Repository.IsArchived}} {{template "repo/issue/view_content/add_reaction" dict "ActionURL" (printf "%s/comments/%d/reactions" $.root.RepoLink .ID)}} {{end}} - {{template "repo/issue/view_content/context_menu" dict "item" . "delete" true "issue" false "diff" true "IsCommentPoster" (and $.root.IsSigned (eq $.root.SignedUserID .PosterID))}} + {{template "repo/issue/view_content/context_menu" dict "ConvContentType" "issue_comment" "ConvContentPage" "pr-files" "ConvContentItem" $comment}}
diff --git a/templates/repo/diff/new_review.tmpl b/templates/repo/diff/new_review.tmpl index b4be2ebae12..513d75cd872 100644 --- a/templates/repo/diff/new_review.tmpl +++ b/templates/repo/diff/new_review.tmpl @@ -1,8 +1,12 @@ -
-
{{end}} +
diff --git a/templates/repo/issue/view_content.tmpl b/templates/repo/issue/view_content.tmpl index dbf2712e100..a04f1cf5940 100644 --- a/templates/repo/issue/view_content.tmpl +++ b/templates/repo/issue/view_content.tmpl @@ -41,7 +41,7 @@ {{if not $.Repository.IsArchived}} {{template "repo/issue/view_content/add_reaction" dict "ActionURL" (printf "%s/issues/%d/reactions" $.RepoLink .Issue.Index)}} {{end}} - {{template "repo/issue/view_content/context_menu" dict "item" .Issue "delete" false "issue" true "diff" false "IsCommentPoster" $.IsIssuePoster}} + {{template "repo/issue/view_content/context_menu" dict "ConvContentType" "issue_main" "ConvContentItem" .Issue}}
diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index c55de5e9743..b82ebade71f 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -56,7 +56,7 @@ {{if not $.Repository.IsArchived}} {{template "repo/issue/view_content/add_reaction" dict "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID)}} {{end}} - {{template "repo/issue/view_content/context_menu" dict "item" . "delete" true "issue" true "diff" false "IsCommentPoster" (and $.IsSigned (eq $.SignedUserID .PosterID))}} + {{template "repo/issue/view_content/context_menu" dict "ConvContentType" "issue_comment" "ConvContentItem" $comment}}
@@ -424,7 +424,7 @@ {{template "repo/issue/view_content/show_role" dict "ShowRole" .ShowRole}} {{if not $.Repository.IsArchived}} {{template "repo/issue/view_content/add_reaction" dict "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID)}} - {{template "repo/issue/view_content/context_menu" dict "item" . "delete" false "issue" true "diff" false "IsCommentPoster" (and $.IsSigned (eq $.SignedUserID .PosterID))}} + {{template "repo/issue/view_content/context_menu" dict "ConvContentType" "pull_review" "ConvContentItem" $comment}} {{end}}
diff --git a/templates/repo/issue/view_content/context_menu.tmpl b/templates/repo/issue/view_content/context_menu.tmpl index 5a482d88bb0..59ee728c509 100644 --- a/templates/repo/issue/view_content/context_menu.tmpl +++ b/templates/repo/issue/view_content/context_menu.tmpl @@ -1,44 +1,45 @@ -
- - {{svg "octicon-kebab-horizontal"}} - +{{$type := $.ConvContentType -}} +{{$item := $.ConvContentItem -}} +{{$page := $.ConvContentPage -}} + +{{$supportDelete := eq $type "issue_comment" -}} +{{$isPagePrFiles := eq $page "pr-files" -}} +{{$issueHTMLURL := ctx.RootData.Issue.HTMLURL ctx -}} +{{$referenceHTMLURL := printf "%s%s#%s" $issueHTMLURL (Iif $isPagePrFiles "/files" "") $item.HashTag -}} + +{{$isCommentPoster := and ctx.RootData.IsSigned (eq ctx.RootData.SignedUserID $item.PosterID) -}} +{{$canEditContent := or ctx.RootData.Permission.IsAdmin ctx.RootData.HasIssuesOrPullsWritePermission $isCommentPoster -}} + +
+ {{svg "octicon-kebab-horizontal"}}
- {{$issueHTMLURL := ctx.RootData.Issue.HTMLURL ctx}} - {{$referenceHTMLURL := ""}} - {{if .issue}}{{/* boolean value: is issue */}} - {{$referenceHTMLURL = printf "%s#%s" $issueHTMLURL .item.HashTag}} - {{else}} - {{$referenceHTMLURL = printf "%s/files#%s" $issueHTMLURL .item.HashTag}} - {{end}} -
{{ctx.Locale.Tr "copy_link"}}
-
{{ctx.Locale.Tr "repo.issues.context.copy_source"}}
+
{{ctx.Locale.Tr "copy_link"}}
+
{{ctx.Locale.Tr "repo.issues.context.copy_source"}}
+ {{if ctx.RootData.IsSigned}} - {{$needDivider := false}} {{if not ctx.RootData.Repository.IsArchived}} - {{$needDivider = true}} -
{{ctx.Locale.Tr "repo.issues.context.quote_reply"}}
+
{{ctx.Locale.Tr "repo.issues.context.quote_reply"}}
{{if not ctx.Consts.RepoUnitTypeIssues.UnitGlobalDisabled}} -
{{ctx.Locale.Tr "repo.issues.context.reference_issue"}}
+
{{ctx.Locale.Tr "repo.issues.context.reference_issue"}}
{{end}} - {{if or ctx.RootData.Permission.IsAdmin .IsCommentPoster ctx.RootData.HasIssuesOrPullsWritePermission}} + {{if $canEditContent}}
-
{{ctx.Locale.Tr "repo.issues.context.edit"}}
- {{if .delete}} -
{{ctx.Locale.Tr "repo.issues.context.delete"}}
+
{{ctx.Locale.Tr "repo.issues.context.edit"}}
+ {{if $supportDelete}} +
{{ctx.Locale.Tr "repo.issues.context.delete"}}
{{end}} {{end}} {{end}} - {{$canUserBlock := call ctx.RootData.CanBlockUser ctx.RootData.SignedUser .item.Poster}} - {{$canOrgBlock := and ctx.RootData.Repository.Owner.IsOrganization (call ctx.RootData.CanBlockUser ctx.RootData.Repository.Owner .item.Poster)}} + + {{$canUserBlock := call ctx.RootData.CanBlockUser ctx.RootData.SignedUser $item.Poster -}} + {{$canOrgBlock := and ctx.RootData.Repository.Owner.IsOrganization (call ctx.RootData.CanBlockUser ctx.RootData.Repository.Owner $item.Poster) -}} {{if or $canOrgBlock $canUserBlock}} - {{if $needDivider}} -
- {{end}} +
{{if $canUserBlock}} -
{{ctx.Locale.Tr "user.block.block.user"}}
+
{{ctx.Locale.Tr "user.block.block.user"}}
{{end}} {{if $canOrgBlock}} -
{{ctx.Locale.Tr "user.block.block.org"}}
+
{{ctx.Locale.Tr "user.block.block.org"}}
{{end}} {{end}} {{end}} diff --git a/templates/repo/issue/view_content/conversation.tmpl b/templates/repo/issue/view_content/conversation.tmpl index 429350fcd77..030315d039d 100644 --- a/templates/repo/issue/view_content/conversation.tmpl +++ b/templates/repo/issue/view_content/conversation.tmpl @@ -4,16 +4,16 @@ At the moment, two kinds of request handler call this template: * Other compare and diff pages -> ... -> {section_unified.tmpl|section_split.tmpl} -> this) The variables in "ctx.Data" are different in each case, making this template fragile, hard to read and maintain. */}} {{if len .comments}} - {{$comment := index .comments 0}} - {{$invalid := $comment.Invalidated}} - {{$resolved := $comment.IsResolved}} - {{$resolveDoer := $comment.ResolveDoer}} - {{$hasReview := and $comment.Review}} - {{$isReviewPending := and $hasReview (eq $comment.Review.Type 0)}} + {{$firstComment := index .comments 0}} + {{$invalid := $firstComment.Invalidated}} + {{$resolved := $firstComment.IsResolved}} + {{$resolveDoer := $firstComment.ResolveDoer}} + {{$hasReview := and $firstComment.Review}} + {{$isReviewPending := and $hasReview (eq $firstComment.Review.Type 0)}}
- {{$comment.TreePath}} + {{$firstComment.TreePath}} {{if $invalid}} {{ctx.Locale.Tr "repo.issues.review.outdated"}} @@ -22,7 +22,7 @@ The variables in "ctx.Data" are different in each case, making this template fra
{{if or $invalid $resolved}} - -
- {{$diff := (CommentMustAsDiff ctx $comment)}} + {{$diff := (CommentMustAsDiff ctx $firstComment)}} {{if $diff}} {{$file := (index $diff.Files 0)}} -
+
@@ -56,9 +56,9 @@ The variables in "ctx.Data" are different in each case, making this template fra {{end}} -
+
- {{range .comments}} + {{range $comment := .comments}} {{$createdSubStr:= DateUtils.TimeSince .CreatedUnix}}
@@ -88,7 +88,7 @@ The variables in "ctx.Data" are different in each case, making this template fra {{template "repo/issue/view_content/show_role" dict "ShowRole" .ShowRole}} {{if not $.Repository.IsArchived}} {{template "repo/issue/view_content/add_reaction" dict "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID)}} - {{template "repo/issue/view_content/context_menu" dict "item" . "delete" true "issue" true "diff" true "IsCommentPoster" (and $.IsSigned (eq $.SignedUserID .PosterID))}} + {{template "repo/issue/view_content/context_menu" dict "ConvContentType" "issue_comment" "ConvContentItem" $comment}} {{end}}
@@ -125,7 +125,7 @@ The variables in "ctx.Data" are different in each case, making this template fra
{{if and $.CanMarkConversation $hasReview (not $isReviewPending)}} -
- {{template "repo/diff/comment_form_datahandler" dict "hidden" true "reply" $comment.ReviewID "root" $ "comment" $comment}} + {{template "repo/diff/comment_form_datahandler" dict "hidden" true "reply" $firstComment.ReviewID "root" $ "comment" $firstComment}}
{{else}} diff --git a/tests/e2e/pr-review.test.ts b/tests/e2e/pr-review.test.ts index 0f64a87976d..d2db212bbb3 100644 --- a/tests/e2e/pr-review.test.ts +++ b/tests/e2e/pr-review.test.ts @@ -36,7 +36,7 @@ test('pr review flow', async ({page, request}) => { request, poster, repoName, - [{path: 'added.txt', content: 'new content\n'}], + [{path: 'added.txt', content: 'new content\nsecond line\n'}], {branch: 'main', newBranch: 'feat', headers: posterHeaders}, ), ]); @@ -80,10 +80,10 @@ test('pr review flow', async ({page, request}) => { await page.goto(`${pullUrl}/files`); - // diff viewer renders the added file with its header and one added-line row + // diff viewer renders the added file with its header and added-line rows const fileBox = page.locator('.diff-file-box[data-new-filename="added.txt"]'); await expect(fileBox.locator('.diff-file-header .file-link')).toHaveText('added.txt'); - await expect(fileBox.locator('tr.add-code')).toHaveCount(1); + await expect(fileBox.locator('tr.add-code')).toHaveCount(2); // commits tab badge reflects the single PR commit, and the diff stats header counts one changed file const commitsTab = page.locator('.ui.pull.tabular.menu a.item', {has: page.locator('.octicon-git-commit')}); @@ -101,11 +101,29 @@ test('pr review flow', async ({page, request}) => { await logout(page); await loginUser(page, reviewer); await page.goto(`${pullUrl}/files`); - await page.locator('#review-box .js-btn-review').click(); + await fileBox.locator('tr.add-code').nth(1).getByRole('button', {name: 'Add line comment', exact: true}).click(); + const pendingForm = fileBox.locator('form').filter({has: page.getByRole('button', {name: 'Start review', exact: true})}); + await pendingForm.getByRole('textbox', {name: 'Leave a comment', exact: true}).fill('pending review comment'); + await pendingForm.getByRole('button', {name: 'Start review', exact: true}).click(); + const counter = page.locator('.review-box-button .review-comments-counter'); + await expect(counter).toHaveAttribute('data-pending-comment-number', '1'); + await expect(counter).toHaveText('1'); + const pendingComment = fileBox.locator('.comment').filter({hasText: 'pending review comment'}); + await expect(pendingComment.locator('[data-clipboard-text]')).toHaveAttribute('data-clipboard-text', /\/files#issuecomment-\d+$/); + await pendingComment.getByRole('menu').filter({has: page.getByRole('menuitem', {name: 'Delete', exact: true, includeHidden: true})}).click(); + page.once('dialog', (dialog) => dialog.accept()); + await pendingComment.getByRole('menuitem', {name: 'Delete', exact: true}).click(); + await expect(pendingComment).toHaveCount(0); + await expect(counter).toHaveAttribute('data-pending-comment-number', '0'); + await expect(counter).toHaveText('0'); + + await page.getByRole('button', {name: 'Review', exact: true}).click(); const panel = page.locator('.review-box-panel'); await panel.locator('textarea[name="content"]').fill(`First approval from ${reviewer}`); await panel.getByRole('button', {name: 'Approve', exact: true}).click(); - await expect(page.locator('.timeline-item').filter({hasText: `First approval from ${reviewer}`})).toBeVisible(); + const reviewComment = page.locator('.timeline-item').filter({hasText: `First approval from ${reviewer}`}); + await expect(reviewComment).toBeVisible(); + await expect(reviewComment.locator('[data-clipboard-text]')).toHaveAttribute('data-clipboard-text', /\/pulls\/\d+#issuecomment-\d+$/); await reRequestAndApprove(reviewer, 'Uncounted approval', 'tw-text-text-light', false); diff --git a/tests/integration/pull_diff_test.go b/tests/integration/pull_diff_test.go index be300ab6a92..60d06e6e043 100644 --- a/tests/integration/pull_diff_test.go +++ b/tests/integration/pull_diff_test.go @@ -40,8 +40,9 @@ func testPullDiffSingleHeadCommitReviewFormAction(t *testing.T) { resp := session.MakeRequest(t, req, http.StatusOK) doc := NewHTMLParser(t, resp.Body) - btn := doc.Find(".js-btn-review") - assert.True(t, btn.Length() == 1 && !btn.HasClass("disabled")) + btn := doc.Find(".review-box-button") + require.Equal(t, 1, btn.Length()) + assert.False(t, btn.Is("[disabled]")) form := doc.Find(".review-box-panel form") assert.Equal(t, 1, form.Length()) assert.Equal(t, "/user2/commitsonpr/pulls/1/files/reviews/submit", form.AttrOr("action", "")) @@ -70,7 +71,9 @@ func testPullDiffAssertPage(t *testing.T, prDiffURL string, reviewBtnDisabled bo }) // Ensure the review button is enabled for full PR reviews - assert.Equal(t, reviewBtnDisabled, doc.Find(".js-btn-review").HasClass("disabled")) + btn := doc.Find(".review-box-button") + require.Equal(t, 1, btn.Length()) + assert.Equal(t, reviewBtnDisabled, btn.Is("[disabled]")) } func TestLongLineDiffRendering(t *testing.T) { diff --git a/web_src/css/review.css b/web_src/css/review.css index 705f3789ba0..0bc1e8e0eef 100644 --- a/web_src/css/review.css +++ b/web_src/css/review.css @@ -131,20 +131,21 @@ max-width: calc(100vw - 70px); /* leave enough space on left, and align the page content */ } -#review-box { +.ui.button.review-box-button { position: relative; + padding-right: 0.5em !important; /* override .repository .diff-detail-box .ui.button */ } -#review-box .review-comments-counter { +.review-box-button .review-comments-counter { background-color: var(--color-primary-light-4); color: var(--color-primary-contrast); } -#review-box:hover .review-comments-counter { +.review-box-button:hover .review-comments-counter { background-color: var(--color-primary-light-5); } -#review-box .review-comments-counter[data-pending-comment-number="0"] { +.review-box-button .review-comments-counter[data-pending-comment-number="0"] { display: none; } diff --git a/web_src/js/features/repo-diff.ts b/web_src/js/features/repo-diff.ts index 35182dafd89..aef00c21e17 100644 --- a/web_src/js/features/repo-diff.ts +++ b/web_src/js/features/repo-diff.ts @@ -77,7 +77,7 @@ function initRepoDiffConversationForm() { // the default behavior is to add a pending review, so if no submitter, it also means "pending_review" if (!submitter || submitter?.matches('button[name="pending_review"]')) { - const reviewBox = document.querySelector('#review-box')!; + const reviewBox = document.querySelector('.review-box-button')!; const counter = reviewBox?.querySelector('.review-comments-counter'); if (!counter) return; const num = parseInt(counter.getAttribute('data-pending-comment-number')!) + 1 || 1; diff --git a/web_src/js/features/repo-issue-edit.test.ts b/web_src/js/features/repo-issue-edit.test.ts index 76a24a64760..5a8b51e101e 100644 --- a/web_src/js/features/repo-issue-edit.test.ts +++ b/web_src/js/features/repo-issue-edit.test.ts @@ -1,10 +1,10 @@ -import {tryShortcutQuoteReply} from './repo-issue-edit.ts'; +import {tryQuoteSelectedReply} from './repo-issue-edit.ts'; -test('tryShortcutQuoteReply', () => { +test('tryQuoteSelectedReply', () => { const container = document.createElement('div'); container.innerHTML = `
- +
Comment text
Raw text
@@ -20,24 +20,24 @@ test('tryShortcutQuoteReply', () => { button.addEventListener('click', onClick); selection.removeAllRanges(); - expect(tryShortcutQuoteReply()).toBe(false); + expect(tryQuoteSelectedReply()).toBe(false); selection.selectAllChildren(container.querySelector('.raw-content')!); - expect(tryShortcutQuoteReply()).toBe(false); + expect(tryQuoteSelectedReply()).toBe(false); const range = document.createRange(); range.selectNodeContents(container.querySelector('.render-content')!.firstChild!); selection.removeAllRanges(); selection.addRange(range); selection.collapseToStart(); - expect(tryShortcutQuoteReply()).toBe(false); + expect(tryQuoteSelectedReply()).toBe(false); expect(onClick).not.toHaveBeenCalled(); selection.removeAllRanges(); selection.addRange(range); - expect(tryShortcutQuoteReply()).toBe(true); + expect(tryQuoteSelectedReply()).toBe(true); expect(onClick).toHaveBeenCalledTimes(1); button.remove(); - expect(tryShortcutQuoteReply()).toBe(false); + expect(tryQuoteSelectedReply()).toBe(false); }); diff --git a/web_src/js/features/repo-issue-edit.ts b/web_src/js/features/repo-issue-edit.ts index 3529f0f9d95..47a62580437 100644 --- a/web_src/js/features/repo-issue-edit.ts +++ b/web_src/js/features/repo-issue-edit.ts @@ -1,4 +1,4 @@ -import {handleReply} from './repo-issue.ts'; +import {handleCommentReply} from './repo-issue.ts'; import {getComboMarkdownEditor, initComboMarkdownEditor, ComboMarkdownEditor} from './comp/ComboMarkdownEditor.ts'; import {POST} from '../modules/fetch.ts'; import {showErrorToast} from '../modules/toast.ts'; @@ -8,12 +8,9 @@ import {triggerUploadStateChanged} from './comp/EditorUpload.ts'; import {convertHtmlToMarkdown} from '../markup/html2markdown.ts'; import {applyAreYouSure} from '../modules/are-you-sure.ts'; import {registerShortcutHandler} from '../modules/shortcut.ts'; +import {registerGlobalEventFunc} from '../modules/observer.ts'; -async function tryOnEditContent(e: Event) { - const clickTarget = (e.target as HTMLElement).closest('.edit-content'); - if (!clickTarget) return; - - e.preventDefault(); +async function onConvContentEdit(clickTarget: HTMLElement) { const commentContent = clickTarget.closest('.comment-header')!.nextElementSibling!; const editContentZone = commentContent.querySelector('.edit-content-zone')!; let renderContent = commentContent.querySelector('.render-content')!; @@ -121,11 +118,7 @@ function extractSelectedMarkdown(container: HTMLElement) { return convertHtmlToMarkdown(el); } -async function tryOnQuoteReply(e: Event) { - const clickTarget = (e.target as HTMLElement).closest('.quote-reply'); - if (!clickTarget) return; - - e.preventDefault(); +async function onConvContentQuoteReply(clickTarget: HTMLElement) { const contentToQuoteId = clickTarget.getAttribute('data-target'); const targetRawToQuote = document.querySelector(`#${contentToQuoteId}.raw-content`)!; const targetMarkupToQuote = targetRawToQuote.parentElement!.querySelector('.render-content.markup')!; @@ -134,9 +127,9 @@ async function tryOnQuoteReply(e: Event) { const quotedContent = `${contentToQuote.replace(/^/mg, '> ')}\n\n`; let editor; - if (clickTarget.classList.contains('quote-reply-diff')) { - const replyBtn = clickTarget.closest('.comment-code-cloud')!.querySelector('button.comment-form-reply')!; - editor = await handleReply(replyBtn); + const btnReply = clickTarget.closest('.comment-code-cloud')?.querySelector('button.comment-form-reply'); + if (btnReply) { + editor = await handleCommentReply(btnReply); } else { // for normal issue/comment page editor = getComboMarkdownEditor(document.querySelector('#comment-form .combo-markdown-editor'))!; @@ -151,7 +144,7 @@ async function tryOnQuoteReply(e: Event) { editor.moveCursorToEnd(); } -export function tryShortcutQuoteReply(): boolean { +export function tryQuoteSelectedReply(): boolean { const selection = window.getSelection(); if (!selection?.rangeCount || selection.isCollapsed) return false; const node = selection.getRangeAt(0).commonAncestorContainer; @@ -160,7 +153,7 @@ export function tryShortcutQuoteReply(): boolean { const commentContainer = el?.closest('.render-content.markup')?.closest('.comment-container'); if (!commentContainer) return false; - const quoteReplyBtn = commentContainer.querySelector('.quote-reply')!; + const quoteReplyBtn = commentContainer.querySelector('[data-shortcut-handler="quoteSelectedReply"]')!; if (!quoteReplyBtn) return false; quoteReplyBtn.click(); @@ -168,15 +161,10 @@ export function tryShortcutQuoteReply(): boolean { } export function initRepoIssueCommentEdit() { - document.addEventListener('click', (e) => { - tryOnEditContent(e); // Edit issue or comment content - tryOnQuoteReply(e); // Quote reply to the comment editor - }); - // TODO: it is not list in the shortcut help modal at the moment, there are many details: - // 1. not every page supports such "r" - // 2. "r" only works when there is a ".quote-reply" menu item, but it is not for a single one - // To make the framework fully support such mechanism correctly, we might also need to consider about dynamically added elements. + registerGlobalEventFunc('click', 'onConvContentQuoteReply', onConvContentQuoteReply); + registerGlobalEventFunc('click', 'onConvContentEdit', onConvContentEdit); + // The shortcut "r" was chosen by https://github.com/go-gitea/gitea/issues/5796#issuecomment-5528657082 - // and GitLab: https://gitlab.com/gitlab-org/gitlab-foss/-/commit/ca5d0c82509cedb94f9bfa4a40e77706a58faafe - registerShortcutHandler('r', tryShortcutQuoteReply); + // The same to GitHub and GitLab: https://gitlab.com/gitlab-org/gitlab-foss/-/commit/ca5d0c82509cedb94f9bfa4a40e77706a58faafe + registerShortcutHandler('quoteSelectedReply', tryQuoteSelectedReply); } diff --git a/web_src/js/features/repo-issue.ts b/web_src/js/features/repo-issue.ts index 1fb2a2ce37e..3c6c0be32bd 100644 --- a/web_src/js/features/repo-issue.ts +++ b/web_src/js/features/repo-issue.ts @@ -17,8 +17,9 @@ import {initRepoIssueSidebar} from './repo-issue-sidebar.ts'; import {fomanticQuery} from '../modules/fomantic/base.ts'; import {showFomanticModal} from '../modules/fomantic/modal.ts'; import {ignoreAreYouSure} from '../modules/are-you-sure.ts'; -import {registerGlobalInitFunc} from '../modules/observer.ts'; +import {registerGlobalEventFunc, registerGlobalInitFunc} from '../modules/observer.ts'; import type {FomanticApiResponse} from '../types.ts'; +import {performFetchActionRequest} from '../modules/fetch-action.ts'; const {appSubUrl} = window.config; @@ -94,69 +95,57 @@ export function initRepoIssueFilterItemLabel() { } export function initRepoIssueCommentDelete() { - // Delete comment - document.addEventListener('click', async (e) => { - if (!(e.target as HTMLElement).matches('.delete-comment')) return; - e.preventDefault(); + registerGlobalEventFunc('click', 'onConvContentDelete', async (deleteButton) => { + if (!window.confirm(deleteButton.getAttribute('data-locale')!)) return; - const deleteButton = e.target as HTMLElement; - if (window.confirm(deleteButton.getAttribute('data-locale')!)) { - try { - const response = await POST(deleteButton.getAttribute('data-url')!); - if (!response.ok) throw new Error('Failed to delete comment'); + const response = await performFetchActionRequest(deleteButton, {method: 'POST', url: deleteButton.getAttribute('data-url')!}); + if (!response) return; - const conversationHolder = deleteButton.closest('.conversation-holder'); - const parentTimelineItem = deleteButton.closest('.timeline-item'); - const parentTimelineGroup = deleteButton.closest('.timeline-item-group'); + const conversationHolder = deleteButton.closest('.conversation-holder'); + const parentTimelineItem = deleteButton.closest('.timeline-item'); + const parentTimelineGroup = deleteButton.closest('.timeline-item-group'); - // Check if this was a pending comment. - if (conversationHolder?.querySelector('.pending-label')) { - const counter = document.querySelector('#review-box .review-comments-counter')!; - let num = parseInt(counter?.getAttribute('data-pending-comment-number') || '') - 1 || 0; - num = Math.max(num, 0); - counter.setAttribute('data-pending-comment-number', String(num)); - counter.textContent = String(num); + // Check if this was a pending comment. + if (conversationHolder?.querySelector('.pending-label')) { + const counter = document.querySelector('.review-box-button .review-comments-counter')!; + let num = parseInt(counter?.getAttribute('data-pending-comment-number') || '') - 1 || 0; + num = Math.max(num, 0); + counter.setAttribute('data-pending-comment-number', String(num)); + counter.textContent = String(num); + } + + document.querySelector(`#${deleteButton.getAttribute('data-comment-id')}`)?.remove(); + + if (conversationHolder && !conversationHolder.querySelector('.comment')) { + const path = conversationHolder.getAttribute('data-path'); + const side = conversationHolder.getAttribute('data-side'); + const idx = conversationHolder.getAttribute('data-idx'); + const lineType = conversationHolder.closest('tr')?.getAttribute('data-line-type'); + + // the conversation holder could appear either on the "Conversation" page, or the "Files Changed" page + // on the Conversation page, there is no parent "tr", so no need to do anything for "add-code-comment" + if (lineType) { + if (lineType === 'same') { + document.querySelector(`[data-path="${CSS.escape(String(path))}"] .add-code-comment[data-idx="${CSS.escape(String(idx))}"]`)!.classList.remove('tw-invisible'); + } else { + document.querySelector(`[data-path="${CSS.escape(String(path))}"] .add-code-comment[data-side="${CSS.escape(String(side))}"][data-idx="${CSS.escape(String(idx))}"]`)!.classList.remove('tw-invisible'); } - - document.querySelector(`#${deleteButton.getAttribute('data-comment-id')}`)?.remove(); - - if (conversationHolder && !conversationHolder.querySelector('.comment')) { - const path = conversationHolder.getAttribute('data-path'); - const side = conversationHolder.getAttribute('data-side'); - const idx = conversationHolder.getAttribute('data-idx'); - const lineType = conversationHolder.closest('tr')?.getAttribute('data-line-type'); - - // the conversation holder could appear either on the "Conversation" page, or the "Files Changed" page - // on the Conversation page, there is no parent "tr", so no need to do anything for "add-code-comment" - if (lineType) { - if (lineType === 'same') { - document.querySelector(`[data-path="${CSS.escape(String(path))}"] .add-code-comment[data-idx="${CSS.escape(String(idx))}"]`)!.classList.remove('tw-invisible'); - } else { - document.querySelector(`[data-path="${CSS.escape(String(path))}"] .add-code-comment[data-side="${CSS.escape(String(side))}"][data-idx="${CSS.escape(String(idx))}"]`)!.classList.remove('tw-invisible'); - } - } - conversationHolder.remove(); - } - - // Check if there is no review content, move the time avatar upward to avoid overlapping the content below. - if (!parentTimelineGroup?.querySelector('.timeline-item.comment') && !parentTimelineItem?.querySelector('.conversation-holder')) { - const timelineAvatar = parentTimelineGroup?.querySelector('.timeline-avatar'); - timelineAvatar?.classList.remove('timeline-avatar-offset'); - } - } catch (error) { - console.error(error); } + conversationHolder.remove(); + } + + // Check if there is no review content, move the time avatar upward to avoid overlapping the content below. + if (!parentTimelineGroup?.querySelector('.timeline-item.comment') && !parentTimelineItem?.querySelector('.conversation-holder')) { + const timelineAvatar = parentTimelineGroup?.querySelector('.timeline-avatar'); + timelineAvatar?.classList.remove('timeline-avatar-offset'); } }); } export function initRepoIssueCodeCommentCancel() { - // Cancel inline code comment - document.addEventListener('click', (e) => { - if (!(e.target as HTMLElement).matches('.cancel-code-comment')) return; - - const form = (e.target as HTMLElement).closest('form')!; - if (form?.classList.contains('comment-form')) { + registerGlobalEventFunc('click', 'onCancelCodeComment', (el) => { + const form = el.closest('form')!; + if (form.classList.contains('comment-form')) { hideElem(form); showElem(form.closest('.comment-code-cloud')!.querySelectorAll('button.comment-form-reply')); } else { @@ -186,7 +175,7 @@ export function initRepoIssueComments() { }); } -export async function handleReply(el: HTMLElement) { +export async function handleCommentReply(el: HTMLElement) { const form = el.closest('.comment-code-cloud')!.querySelector('.comment-form')!; const textarea = form.querySelector('textarea'); @@ -242,15 +231,13 @@ export function initRepoPullRequestReview() { addDelegatedEventListener(document, 'click', 'button.comment-form-reply', (el, e) => { e.preventDefault(); - handleReply(el); + handleCommentReply(el); }); - // The following part is only for diff views - if (!document.querySelector('.repository.pull.diff')) return; - - const elReviewBtn = document.querySelector('.js-btn-review'); - const elReviewPanel = document.querySelector('.review-box-panel.tippy-target'); - if (elReviewBtn && elReviewPanel) { + registerGlobalInitFunc('initPullReviewBox', (el) => { + const elReviewBtn = el.querySelector('.review-box-button')!; + const elReviewPanel = el.querySelector('.review-box-panel.tippy-target'); + if (!elReviewPanel) return; const tippy = createTippy(elReviewBtn, { content: elReviewPanel, theme: 'default', @@ -260,8 +247,8 @@ export function initRepoPullRequestReview() { interactive: true, hideOnClick: true, }); - elReviewPanel.querySelector('.close')!.addEventListener('click', () => tippy.hide()); - } + elReviewPanel.querySelector('.close-review-panel')!.addEventListener('click', () => tippy.hide()); + }); addDelegatedEventListener(document, 'click', '.add-code-comment', async (el, e) => { e.preventDefault(); diff --git a/web_src/js/features/repo-legacy.ts b/web_src/js/features/repo-legacy.ts index 3bda12c9819..9656604790f 100644 --- a/web_src/js/features/repo-legacy.ts +++ b/web_src/js/features/repo-legacy.ts @@ -42,13 +42,13 @@ export function initRepository() { registerGlobalInitFunc('initRepoPullRequestUpdate', initRepoPullRequestUpdate); registerGlobalInitFunc('initRepoCitationPanel', initRepoCitationPanel); + // TODO: need to refactor more legacy if-querySelector-based "init" code to "data-global-xxx" framework in the future. const pageContent = document.querySelector('.page-content.repository'); if (!pageContent) return; initRepoBranchTagSelector(); initRepoCommentFormAndSidebar(); - // Labels initCompLabelEdit('.page-content.repository.labels'); initRepoMilestone(); initRepoNew(); @@ -57,18 +57,17 @@ export function initRepository() { initRepoSettings(); initRepoIssueWipNewTitle(); - // Issues - if (pageContent.matches('.page-content.repository.view.issue')) { - initRepoIssueCommentEdit(); + initRepoIssueCommentEdit(); + initRepoIssueCommentDelete(); + initRepoIssueCodeCommentCancel(); + if (pageContent.matches('.page-content.repository.view.issue')) { initRepoIssueTitleEdit(); initRepoIssueWipToggle(); initRepoIssueComments(); initRepoIssueReferenceIssue(); - initRepoIssueCommentDelete(); - initRepoIssueCodeCommentCancel(); initCompReactionSelector(); } diff --git a/web_src/js/modules/shortcut.ts b/web_src/js/modules/shortcut.ts index 4be22b1c0ad..58d2ac26a4a 100644 --- a/web_src/js/modules/shortcut.ts +++ b/web_src/js/modules/shortcut.ts @@ -1,7 +1,7 @@ import {registerGlobalInitFunc} from './observer.ts'; import {createElementFromHTML, hideElem, toggleElem} from '../utils/dom.ts'; import {showFomanticModal} from './fomantic/modal.ts'; -import {html} from '../utils/html.ts'; +import {html, htmlRaw} from '../utils/html.ts'; type ShortcutHandler = (el: HTMLElement) => boolean; @@ -87,6 +87,7 @@ function showShortcutHelp() { document.body.append(modal); } + const seenShortcuts = new Set(); const list = modal.querySelector('.shortcut-list')!; list.replaceChildren(createElementFromHTML(html``)); for (const elem of document.querySelectorAll('[data-shortcut-keys]')) { @@ -99,15 +100,24 @@ function showShortcutHelp() { target.getAttribute('placeholder') || target.getAttribute('title') || getShortcutTextContent(target); - list.append(createElementFromHTML(html``)); + + const shortcutKeys = elem.getAttribute('data-shortcut-keys')!; + const shortcutHandler = elem.getAttribute('data-shortcut-handler') ?? ''; + const shortcutIdentifier = `${shortcutKeys}::${shortcutHandler}`; + if (seenShortcuts.has(shortcutIdentifier)) continue; + seenShortcuts.add(shortcutIdentifier); + let keysHtml = ''; + for (const keys of shortcutKeys.split(' ')) { + keysHtml += `${keys} `; + } + list.append(createElementFromHTML(html``)); } showFomanticModal(modal); } -export function registerShortcutHandler(fullKey: string, handler: ShortcutHandler) { - fullKey = fullKey.toLowerCase(); - shortcutHandlers[fullKey] ||= []; - shortcutHandlers[fullKey].push(handler); +export function registerShortcutHandler(name: string, handler: ShortcutHandler) { + shortcutHandlers[name] ||= []; + shortcutHandlers[name].push(handler); } export function initGlobalShortcut() { @@ -169,20 +179,22 @@ export function initGlobalShortcut() { shortcutPresses.push({fullKey, pressTime}); const fullKeys = shortcutPresses.map((p) => p.fullKey).join(' '); - for (const handler of shortcutHandlers[fullKeys.toLowerCase()] || []) { - if (handler(target)) { - e.preventDefault(); - shortcutPresses.length = 0; - return; - } - } - // It's said that "spaces in aria-keyshortcuts separate alternative shortcuts", but we want to support multi-key shortcuts // So here we use our own data attribute to store the full shortcut keys, and match it case-insensitively. const matchedElems = document.querySelectorAll(`[data-shortcut-keys="${CSS.escape(fullKeys)}" i]`); if (!matchedElems.length) return; // TODO: if there are multiple matches, maybe we could introduce some priority rules to determine which one to trigger const matchedElem = matchedElems[0]; + const handlerName = matchedElem.getAttribute('data-shortcut-handler')!; + if (shortcutHandlers[handlerName]) { + for (const handler of shortcutHandlers[handlerName]) { + if (handler(target)) break; + } + e.preventDefault(); + shortcutPresses.length = 0; + return; + } + e.preventDefault(); shortcutPresses.length = 0; matchedElem.click();
?Keyboard Shortcuts
${elem.getAttribute('data-shortcut-keys')}${label}
${htmlRaw(keysHtml)}${label}