From 0352b992218d21af5937c11e52fb303f5792d607 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Sun, 14 Apr 2024 19:58:48 +0800 Subject: [PATCH] Rewrite and restyle reaction selector and enable no-sizzle eslint rule (#30453) (#30473) Backport #30453 by @silverwind Enable `no-sizzle` lint rule, there was only one use in `initCompReactionSelector` which I have rewritten as follows: - Remove all jQuery except the necessary fomantic dropdown init - Remove the recursion, instead bind event listeners to common parent container nodes Did various tests, works with our without attachments, in diff view and in diff comments inside comment list. Additionally the style of reactions now matches between code comments and issue comments: Screenshot 2024-04-13 at 14 58 10 Co-authored-by: silverwind Co-authored-by: wxiaoguang --- .eslintrc.yaml | 4 +- routers/web/repo/issue.go | 2 - services/context/context.go | 1 + templates/repo/diff/comments.tmpl | 4 +- templates/repo/issue/view_content.tmpl | 4 +- .../repo/issue/view_content/add_reaction.tmpl | 10 +- .../repo/issue/view_content/comments.tmpl | 8 +- .../repo/issue/view_content/conversation.tmpl | 8 +- .../repo/issue/view_content/reactions.tmpl | 8 +- web_src/css/base.css | 6 +- web_src/css/index.css | 1 + web_src/css/modules/comment.css | 2 +- web_src/css/repo.css | 124 ++---------------- web_src/css/repo/reactions.css | 70 ++++++++++ web_src/js/features/comp/ReactionSelector.js | 56 ++++---- web_src/js/features/repo-diff.js | 1 - web_src/js/features/repo-legacy.js | 2 +- 17 files changed, 133 insertions(+), 178 deletions(-) create mode 100644 web_src/css/repo/reactions.css diff --git a/.eslintrc.yaml b/.eslintrc.yaml index 99ce2e97d67..bc6376d979f 100644 --- a/.eslintrc.yaml +++ b/.eslintrc.yaml @@ -317,7 +317,7 @@ rules: jquery/no-serialize: [2] jquery/no-show: [2] jquery/no-size: [2] - jquery/no-sizzle: [0] + jquery/no-sizzle: [2] jquery/no-slide: [0] jquery/no-submit: [0] jquery/no-text: [0] @@ -469,7 +469,7 @@ rules: no-jquery/no-selector-prop: [2] no-jquery/no-serialize: [2] no-jquery/no-size: [2] - no-jquery/no-sizzle: [0] + no-jquery/no-sizzle: [2] no-jquery/no-slide: [2] no-jquery/no-sub: [2] no-jquery/no-support: [2] diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index e4f2e9a2bc2..1364d756766 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -3318,7 +3318,6 @@ func ChangeIssueReaction(ctx *context.Context) { } html, err := ctx.RenderToHTML(tplReactions, map[string]any{ - "ctxData": ctx.Data, "ActionURL": fmt.Sprintf("%s/issues/%d/reactions", ctx.Repo.RepoLink, issue.Index), "Reactions": issue.Reactions.GroupByType(), }) @@ -3425,7 +3424,6 @@ func ChangeCommentReaction(ctx *context.Context) { } html, err := ctx.RenderToHTML(tplReactions, map[string]any{ - "ctxData": ctx.Data, "ActionURL": fmt.Sprintf("%s/comments/%d/reactions", ctx.Repo.RepoLink, comment.ID), "Reactions": comment.Reactions.GroupByType(), }) diff --git a/services/context/context.go b/services/context/context.go index 4b318f7e338..f591414f213 100644 --- a/services/context/context.go +++ b/services/context/context.go @@ -102,6 +102,7 @@ func NewTemplateContextForWeb(ctx *Context) TemplateContext { tmplCtx := NewTemplateContext(ctx) tmplCtx["Locale"] = ctx.Base.Locale tmplCtx["AvatarUtils"] = templates.NewAvatarUtils(ctx) + tmplCtx["RootData"] = ctx.Data return tmplCtx } diff --git a/templates/repo/diff/comments.tmpl b/templates/repo/diff/comments.tmpl index a9120465bdc..c7f43371824 100644 --- a/templates/repo/diff/comments.tmpl +++ b/templates/repo/diff/comments.tmpl @@ -48,7 +48,7 @@ {{end}} {{end}} - {{template "repo/issue/view_content/add_reaction" dict "ctxData" $.root "ActionURL" (printf "%s/comments/%d/reactions" $.root.RepoLink .ID)}} + {{template "repo/issue/view_content/add_reaction" dict "ActionURL" (printf "%s/comments/%d/reactions" $.root.RepoLink .ID)}} {{template "repo/issue/view_content/context_menu" dict "ctxData" $.root "item" . "delete" true "issue" false "diff" true "IsCommentPoster" (and $.root.IsSigned (eq $.root.SignedUserID .PosterID))}} @@ -68,7 +68,7 @@ {{$reactions := .Reactions.GroupByType}} {{if $reactions}} - {{template "repo/issue/view_content/reactions" dict "ctxData" $.root "ActionURL" (printf "%s/comments/%d/reactions" $.root.RepoLink .ID) "Reactions" $reactions}} + {{template "repo/issue/view_content/reactions" dict "ActionURL" (printf "%s/comments/%d/reactions" $.root.RepoLink .ID) "Reactions" $reactions}} {{end}} diff --git a/templates/repo/issue/view_content.tmpl b/templates/repo/issue/view_content.tmpl index c65b79dea70..06d0586683b 100644 --- a/templates/repo/issue/view_content.tmpl +++ b/templates/repo/issue/view_content.tmpl @@ -46,7 +46,7 @@
{{template "repo/issue/view_content/show_role" dict "ShowRole" .Issue.ShowRole "IgnorePoster" true}} {{if not $.Repository.IsArchived}} - {{template "repo/issue/view_content/add_reaction" dict "ctxData" $ "ActionURL" (printf "%s/issues/%d/reactions" $.RepoLink .Issue.Index)}} + {{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 "ctxData" $ "item" .Issue "delete" false "issue" true "diff" false "IsCommentPoster" $.IsIssuePoster}}
@@ -67,7 +67,7 @@ {{$reactions := .Issue.Reactions.GroupByType}} {{if $reactions}} - {{template "repo/issue/view_content/reactions" dict "ctxData" $ "ActionURL" (printf "%s/issues/%d/reactions" $.RepoLink .Issue.Index) "Reactions" $reactions}} + {{template "repo/issue/view_content/reactions" dict "ActionURL" (printf "%s/issues/%d/reactions" $.RepoLink .Issue.Index) "Reactions" $reactions}} {{end}} diff --git a/templates/repo/issue/view_content/add_reaction.tmpl b/templates/repo/issue/view_content/add_reaction.tmpl index 37931f287e3..6baded8fe94 100644 --- a/templates/repo/issue/view_content/add_reaction.tmpl +++ b/templates/repo/issue/view_content/add_reaction.tmpl @@ -1,11 +1,9 @@ -{{if .ctxData.IsSigned}} +{{if ctx.RootData.IsSigned}}
- - {{svg "octicon-smiley"}} - -
+ {{svg "octicon-smiley"}} +
{{range $value := AllowedReactions}} - {{ReactionToEmoji $value}} + {{ReactionToEmoji $value}} {{end}}
diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index d900d23c471..acc04e4c615 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -53,7 +53,7 @@
{{template "repo/issue/view_content/show_role" dict "ShowRole" .ShowRole}} {{if not $.Repository.IsArchived}} - {{template "repo/issue/view_content/add_reaction" dict "ctxData" $ "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID)}} + {{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 "ctxData" $ "item" . "delete" true "issue" true "diff" false "IsCommentPoster" (and $.IsSigned (eq $.SignedUserID .PosterID))}}
@@ -74,7 +74,7 @@
{{$reactions := .Reactions.GroupByType}} {{if $reactions}} - {{template "repo/issue/view_content/reactions" dict "ctxData" $ "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID) "Reactions" $reactions}} + {{template "repo/issue/view_content/reactions" dict "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID) "Reactions" $reactions}} {{end}} @@ -427,7 +427,7 @@
{{template "repo/issue/view_content/show_role" dict "ShowRole" .ShowRole}} {{if not $.Repository.IsArchived}} - {{template "repo/issue/view_content/add_reaction" dict "ctxData" $ "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID)}} + {{template "repo/issue/view_content/add_reaction" dict "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID)}} {{template "repo/issue/view_content/context_menu" dict "ctxData" $ "item" . "delete" false "issue" true "diff" false "IsCommentPoster" (and $.IsSigned (eq $.SignedUserID .PosterID))}} {{end}}
@@ -448,7 +448,7 @@ {{$reactions := .Reactions.GroupByType}} {{if $reactions}} - {{template "repo/issue/view_content/reactions" dict "ctxData" $ "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID) "Reactions" $reactions}} + {{template "repo/issue/view_content/reactions" dict "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID) "Reactions" $reactions}} {{end}} diff --git a/templates/repo/issue/view_content/conversation.tmpl b/templates/repo/issue/view_content/conversation.tmpl index 79e7cb498bc..ac32a2db5d2 100644 --- a/templates/repo/issue/view_content/conversation.tmpl +++ b/templates/repo/issue/view_content/conversation.tmpl @@ -55,8 +55,8 @@
{{range .comments}} {{$createdSubStr:= TimeSinceUnix .CreatedUnix ctx.Locale}} -
-
+
+
{{if not .OriginalAuthor}} @@ -82,7 +82,7 @@
{{template "repo/issue/view_content/show_role" dict "ShowRole" .ShowRole}} {{if not $.Repository.IsArchived}} - {{template "repo/issue/view_content/add_reaction" dict "ctxData" $ "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID)}} + {{template "repo/issue/view_content/add_reaction" dict "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID)}} {{template "repo/issue/view_content/context_menu" dict "ctxData" $ "item" . "delete" true "issue" true "diff" true "IsCommentPoster" (and $.IsSigned (eq $.SignedUserID .PosterID))}} {{end}}
@@ -103,7 +103,7 @@
{{$reactions := .Reactions.GroupByType}} {{if $reactions}} - {{template "repo/issue/view_content/reactions" dict "ctxData" $ "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID) "Reactions" $reactions}} + {{template "repo/issue/view_content/reactions" dict "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID) "Reactions" $reactions}} {{end}}
diff --git a/templates/repo/issue/view_content/reactions.tmpl b/templates/repo/issue/view_content/reactions.tmpl index da6319667bf..0011efe8b28 100644 --- a/templates/repo/issue/view_content/reactions.tmpl +++ b/templates/repo/issue/view_content/reactions.tmpl @@ -1,7 +1,7 @@ -
+
{{range $key, $value := .Reactions}} - {{$hasReacted := $value.HasUser $.ctxData.SignedUserID}} - {{end}} {{if AllowedReactions}} - {{template "repo/issue/view_content/add_reaction" dict "ctxData" $.ctxData "ActionURL" .ActionURL}} + {{template "repo/issue/view_content/add_reaction" dict "ActionURL" .ActionURL}} {{end}}
diff --git a/web_src/css/base.css b/web_src/css/base.css index cf85e118d9b..4707bb8f804 100644 --- a/web_src/css/base.css +++ b/web_src/css/base.css @@ -1440,8 +1440,7 @@ table th[data-sortt-desc] .svg { box-shadow: 0 0 0 1px var(--color-secondary) inset; } -.emoji, -.reaction { +.emoji { font-size: 1.25em; line-height: var(--line-height-default); font-style: normal !important; @@ -1449,8 +1448,7 @@ table th[data-sortt-desc] .svg { vertical-align: -0.075em; } -.emoji img, -.reaction img { +.emoji img { border-width: 0 !important; margin: 0 !important; width: 1em !important; diff --git a/web_src/css/index.css b/web_src/css/index.css index 09246b79c0b..a6c08c9a301 100644 --- a/web_src/css/index.css +++ b/web_src/css/index.css @@ -60,6 +60,7 @@ @import "./repo/linebutton.css"; @import "./repo/wiki.css"; @import "./repo/header.css"; +@import "./repo/reactions.css"; @import "./editor/fileeditor.css"; @import "./editor/combomarkdowneditor.css"; diff --git a/web_src/css/modules/comment.css b/web_src/css/modules/comment.css index 799eeb8574d..cf080db2255 100644 --- a/web_src/css/modules/comment.css +++ b/web_src/css/modules/comment.css @@ -16,7 +16,7 @@ .ui.comments .comment { position: relative; background: none; - margin: 0.5em 0 0; + margin: 3px 0 0; padding: 0.5em 0 0; border: none; border-top: none; diff --git a/web_src/css/repo.css b/web_src/css/repo.css index 36cebb016b1..bd18f9ba57c 100644 --- a/web_src/css/repo.css +++ b/web_src/css/repo.css @@ -913,6 +913,9 @@ td .commit-summary { .repository.view.issue .comment-list .ui.comments { max-width: 100%; + display: flex; + flex-direction: column; + gap: 3px; } .repository.view.issue .comment-list .comment > .content > div:first-child { @@ -928,6 +931,11 @@ td .commit-summary { .repository.view.issue .comment-list .comment .comment-container { border: 1px solid var(--color-secondary); border-radius: var(--border-radius); + background: var(--color-box-body); +} + +.repository.view.issue .comment-list .conversation-holder .comment .comment-container { + border: none; } @media (max-width: 767.98px) { @@ -1042,30 +1050,6 @@ td .commit-summary { margin-left: 42px; } -.repository.view.issue .comment-list .comment-code-cloud .segment.reactions { - margin-top: 16px !important; - margin-bottom: -8px !important; - border-top: none !important; -} - -.repository.view.issue .comment-list .comment-code-cloud .segment.reactions .ui.label { - border: 1px solid; - padding: 5px 8px !important; - margin: 0 2px; - border-radius: var(--border-radius); - border-color: var(--color-secondary-dark-1) !important; -} - -.repository.view.issue .comment-list .comment-code-cloud .segment.reactions .ui.label.basic.primary { - background-color: var(--color-reaction-active-bg) !important; - border-color: var(--color-primary-alpha-80) !important; -} - -.repository.view.issue .comment-list .comment-code-cloud .segment.reactions .ui.label.basic.primary:hover { - background-color: var(--color-reaction-hover-bg) !important; - border-color: var(--color-primary-alpha-80) !important; -} - .repository.view.issue .comment-list .comment-code-cloud button.comment-form-reply { margin: 0; } @@ -1902,98 +1886,6 @@ td .commit-summary { border-bottom: 1px solid var(--color-warning-border); } -.repository .segment.reactions.dropdown .menu, -.repository .select-reaction.dropdown .menu { - right: 0 !important; - left: auto !important; - min-width: 170px; -} - -.repository .segment.reactions.dropdown .menu > .header, -.repository .select-reaction.dropdown .menu > .header { - margin: 0.75rem 0 0.5rem; -} - -.repository .segment.reactions.dropdown .menu > .item, -.repository .select-reaction.dropdown .menu > .item { - float: left; - margin: 4px; - font-size: 20px; - width: 34px; - height: 34px; - min-height: 0 !important; - border-radius: var(--border-radius); - display: flex !important; - align-items: center; - justify-content: center; -} - -.repository .segment.reactions { - padding: 0; - display: flex; - border: none !important; - border-top: 1px solid var(--color-secondary) !important; - width: 100% !important; - max-width: 100% !important; - margin: 0 !important; - border-radius: 0 0 var(--border-radius) var(--border-radius); -} - -.repository .segment.reactions .ui.label { - max-height: 40px; - padding: 8px 16px !important; - display: flex !important; - align-items: center; - border: 0; - border-right: 1px solid; - border-radius: 0; - margin: 0; - font-size: 12px; - font-weight: var(--font-weight-normal); - border-color: var(--color-secondary) !important; - background: var(--color-reaction-bg); -} - -.repository .segment.reactions .ui.label:first-of-type { - border-bottom-left-radius: 3px; -} - -.repository .segment.reactions .ui.label.disabled { - cursor: default; - opacity: 1; -} - -.repository .segment.reactions .ui.label.basic.primary { - color: var(--color-primary) !important; - background-color: var(--color-reaction-active-bg) !important; - border-color: var(--color-secondary-dark-1) !important; -} - -.repository .segment.reactions .ui.label.basic:hover { - background-color: var(--color-reaction-hover-bg) !important; -} - -.repository .segment.reactions .reaction-count { - margin-left: 0.5rem; -} - -.repository .segment.reactions .select-reaction { - display: flex; - align-items: center; -} - -.repository .segment.reactions .select-reaction a { - padding: 0 14px; -} - -.repository .segment.reactions .select-reaction:not(.active) a { - display: none; -} - -.repository .segment.reactions:hover .select-reaction a { - display: block; -} - .repository .ui.fluid.action.input .ui.search.action.input { flex: auto; } diff --git a/web_src/css/repo/reactions.css b/web_src/css/repo/reactions.css new file mode 100644 index 00000000000..8fe01af4f06 --- /dev/null +++ b/web_src/css/repo/reactions.css @@ -0,0 +1,70 @@ +.bottom-reactions { + display: flex; + gap: 6px; + margin: 0 1em 1em; +} + +.timeline-item .conversation-holder .bottom-reactions { + margin: 1em 0 0 36px; + padding-bottom: 8px; +} + +.bottom-reactions .ui.label { + padding: 5px 8px; + font-weight: var(--font-weight-normal); +} + +.bottom-reactions .ui.label.primary { + background-color: var(--color-reaction-active-bg) !important; +} + +.bottom-reactions .ui.label:hover { + background-color: var(--color-reaction-hover-bg) !important; +} + +.bottom-reactions .ui.label.disabled { + cursor: default; + opacity: 1; +} + +.bottom-reactions .ui.label .reaction { + font-size: 16px; + display: flex; +} + +.bottom-reactions .ui.label .reaction img { + height: 16px; + aspect-ratio: 1; +} + +.bottom-reactions .reaction-count { + margin-left: 4px; +} + +.ui.dropdown.select-reaction .menu { + min-width: 170px; /* item-outer-width * 4 */ +} + +.ui.dropdown.select-reaction .menu > .item { + float: left; + margin: 4px; + font-size: 20px; + width: 34px; + height: 34px; + border-radius: var(--border-radius); + display: flex; + align-items: center; + justify-content: center; +} + +.bottom-reactions .select-reaction { + padding: 0 10px; +} + +.bottom-reactions .select-reaction:not(.active) { + visibility: hidden; +} + +.bottom-reactions:hover .select-reaction { + visibility: visible; +} diff --git a/web_src/js/features/comp/ReactionSelector.js b/web_src/js/features/comp/ReactionSelector.js index 2def3db51a7..e507b89632a 100644 --- a/web_src/js/features/comp/ReactionSelector.js +++ b/web_src/js/features/comp/ReactionSelector.js @@ -1,38 +1,36 @@ import $ from 'jquery'; import {POST} from '../../modules/fetch.js'; -export function initCompReactionSelector($parent) { - $parent.find(`.select-reaction .item.reaction, .comment-reaction-button`).on('click', async function (e) { - e.preventDefault(); +export function initCompReactionSelector() { + for (const container of document.querySelectorAll('.issue-content, .diff-file-body')) { + container.addEventListener('click', async (e) => { + // there are 2 places for the "reaction" buttons, one is the top-right reaction menu, one is the bottom of the comment + const target = e.target.closest('.comment-reaction-button'); + if (!target) return; + e.preventDefault(); - if (this.classList.contains('disabled')) return; + if (target.classList.contains('disabled')) return; - const actionUrl = this.closest('[data-action-url]')?.getAttribute('data-action-url'); - const reactionContent = this.getAttribute('data-reaction-content'); - const hasReacted = this.closest('.ui.segment.reactions')?.querySelector(`a[data-reaction-content="${reactionContent}"]`)?.getAttribute('data-has-reacted') === 'true'; + const actionUrl = target.closest('[data-action-url]').getAttribute('data-action-url'); + const reactionContent = target.getAttribute('data-reaction-content'); - const res = await POST(`${actionUrl}/${hasReacted ? 'unreact' : 'react'}`, { - data: new URLSearchParams({content: reactionContent}), - }); + const commentContainer = target.closest('.comment-container'); - const data = await res.json(); - if (data && (data.html || data.empty)) { - const $content = $(this).closest('.content'); - let $react = $content.find('.segment.reactions'); - if ((!data.empty || data.html === '') && $react.length > 0) { - $react.remove(); - } - if (!data.empty) { - const $attachments = $content.find('.segment.bottom:first'); - $react = $(data.html); - if ($attachments.length > 0) { - $react.insertBefore($attachments); - } else { - $react.appendTo($content); - } - $react.find('.dropdown').dropdown(); - initCompReactionSelector($react); + const bottomReactions = commentContainer.querySelector('.bottom-reactions'); // may not exist if there is no reaction + const bottomReactionBtn = bottomReactions?.querySelector(`a[data-reaction-content="${CSS.escape(reactionContent)}"]`); + const hasReacted = bottomReactionBtn?.getAttribute('data-has-reacted') === 'true'; + + const res = await POST(`${actionUrl}/${hasReacted ? 'unreact' : 'react'}`, { + data: new URLSearchParams({content: reactionContent}), + }); + + const data = await res.json(); + bottomReactions?.remove(); + if (data.html) { + commentContainer.insertAdjacentHTML('beforeend', data.html); + const bottomReactionsDropdowns = commentContainer.querySelectorAll('.bottom-reactions .dropdown.select-reaction'); + $(bottomReactionsDropdowns).dropdown(); // re-init the dropdown } - } - }); + }); + } } diff --git a/web_src/js/features/repo-diff.js b/web_src/js/features/repo-diff.js index 596b1ea3805..3610711c0af 100644 --- a/web_src/js/features/repo-diff.js +++ b/web_src/js/features/repo-diff.js @@ -80,7 +80,6 @@ function initRepoDiffConversationForm() { $(`[data-path="${path}"] .add-code-comment[data-side="${side}"][data-idx="${idx}"]`).addClass('tw-invisible'); } $newConversationHolder.find('.dropdown').dropdown(); - initCompReactionSelector($newConversationHolder); } catch (error) { console.error('Error:', error); showErrorToast(i18n.network_error); diff --git a/web_src/js/features/repo-legacy.js b/web_src/js/features/repo-legacy.js index e83de27e4cf..18d98c891d0 100644 --- a/web_src/js/features/repo-legacy.js +++ b/web_src/js/features/repo-legacy.js @@ -393,7 +393,7 @@ export function initRepository() { initRepoIssueDependencyDelete(); initRepoIssueCodeCommentCancel(); initRepoPullRequestUpdate(); - initCompReactionSelector($(document)); + initCompReactionSelector(); initRepoPullRequestMergeForm(); initRepoPullRequestCommitStatus();