From 4b1063f3dba6ef7a54c15f6e795409b504a62391 Mon Sep 17 00:00:00 2001 From: silverwind Date: Sun, 14 Apr 2024 12:44:11 +0200 Subject: [PATCH] Rewrite and restyle reaction selector and enable no-sizzle eslint rule (#30453) Enable `no-sizzle` lint rule, there was only one use in `initCompReactionSelector` and: - Remove all jQuery except the necessary fomantic dropdown init - Remove the recursion, instead bind event listeners to common parent container nodes --------- Co-authored-by: wxiaoguang Co-authored-by: Giteabot --- .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 | 58 ++++---- web_src/js/features/repo-diff.js | 1 - web_src/js/features/repo-legacy.js | 2 +- 17 files changed, 134 insertions(+), 179 deletions(-) create mode 100644 web_src/css/repo/reactions.css diff --git a/.eslintrc.yaml b/.eslintrc.yaml index 5fd0a245f..3e4c6ea50 100644 --- a/.eslintrc.yaml +++ b/.eslintrc.yaml @@ -318,7 +318,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] @@ -470,7 +470,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 e4f2e9a2b..1364d7567 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 7ab48afb7..9d7787bf4 100644 --- a/services/context/context.go +++ b/services/context/context.go @@ -101,6 +101,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 a9120465b..c7f433718 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 c65b79dea..06d058668 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 37931f287..6baded8fe 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 d900d23c4..acc04e4c6 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 79e7cb498..ac32a2db5 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 da6319667..0011efe8b 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 c6a22a5dc..02067971a 100644 --- a/web_src/css/base.css +++ b/web_src/css/base.css @@ -1366,8 +1366,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; @@ -1375,8 +1374,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 ad59f3263..73e10eedc 100644 --- a/web_src/css/index.css +++ b/web_src/css/index.css @@ -62,6 +62,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 799eeb857..cf080db22 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 edb3bc2e8..4b7ad4967 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 000000000..8fe01af4f --- /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 2def3db51..e507b8963 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 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 + } }); - - 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); - } - } - }); + } } diff --git a/web_src/js/features/repo-diff.js b/web_src/js/features/repo-diff.js index b2e8ec486..00f74515d 100644 --- a/web_src/js/features/repo-diff.js +++ b/web_src/js/features/repo-diff.js @@ -87,7 +87,6 @@ function initRepoDiffConversationForm() { el.classList.add('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 e83de27e4..18d98c891 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();