From 595e8abd686c7d555ff94668fb46604b92025956 Mon Sep 17 00:00:00 2001 From: silverwind Date: Sun, 28 May 2023 03:34:18 +0200 Subject: [PATCH] Improve and fix bugs surrounding reactions (#24760) - Slightly decrease size of reaction buttons - Remove tooltip inside menu, it's obvious by the picture alone - Fix top menu triangle - Use `display: grid` to align icons in menu - Use regular tooltip for reaction users - Fix bug that deleted the reaction bar on clicking already reacted reaction in dropdown Screenshot 2023-05-17 at 00 03 42 Screenshot 2023-05-17 at 00 11 14 Screenshot 2023-05-17 at 00 29 30 Screenshot 2023-05-17 at 00 29 54 Screenshot 2023-05-17 at 00 12 20 --------- Co-authored-by: wxiaoguang Co-authored-by: Giteabot --- options/locale/locale_en-US.ini | 1 - templates/repo/diff/comments.tmpl | 6 +- templates/repo/issue/view_content.tmpl | 8 +- .../repo/issue/view_content/add_reaction.tmpl | 8 +- .../repo/issue/view_content/comments.tmpl | 12 +-- .../repo/issue/view_content/context_menu.tmpl | 2 +- .../repo/issue/view_content/reactions.tmpl | 8 +- web_src/css/base.css | 19 ++++- web_src/css/repo.css | 69 ++++++++--------- web_src/css/themes/theme-arc-green.css | 7 +- web_src/js/features/comp/ReactionSelector.js | 75 +++++++++---------- web_src/js/features/repo-code.js | 4 +- web_src/js/features/repo-issue-content.js | 2 +- web_src/js/features/repo-legacy.js | 2 +- web_src/js/modules/tippy.js | 7 ++ 15 files changed, 117 insertions(+), 113 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 1026a13e3..16dbb0948 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -965,7 +965,6 @@ mirror_password_help = Change the username to erase a stored password. watchers = Watchers stargazers = Stargazers forks = Forks -pick_reaction = Pick your reaction reactions_more = and %d more unit_disabled = The site administrator has disabled this repository section. language_other = Other diff --git a/templates/repo/diff/comments.tmpl b/templates/repo/diff/comments.tmpl index 50f0c903a..5b0b32cc9 100644 --- a/templates/repo/diff/comments.tmpl +++ b/templates/repo/diff/comments.tmpl @@ -15,7 +15,7 @@ {{svg (MigrationIcon $.root.Repository.GetOriginalURLHostname)}} {{.OriginalAuthor}} - + {{$.root.locale.Tr "repo.issues.commented_at" (.HashTag|Escape) $createdStr | Safe}} @@ -24,7 +24,7 @@ {{end}} {{else}} - + {{template "shared/user/namelink" .Poster}} {{$.root.locale.Tr "repo.issues.commented_at" (.HashTag|Escape) $createdStr | Safe}} @@ -59,9 +59,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}} -
{{end}} diff --git a/templates/repo/issue/view_content.tmpl b/templates/repo/issue/view_content.tmpl index 3c60eca67..77087e0a8 100644 --- a/templates/repo/issue/view_content.tmpl +++ b/templates/repo/issue/view_content.tmpl @@ -27,7 +27,7 @@ {{svg (MigrationIcon .Repository.GetOriginalURLHostname)}} {{.Issue.OriginalAuthor}} - + {{.locale.Tr "repo.issues.commented_at" (.Issue.HashTag|Escape) $createdStr | Safe}} @@ -37,7 +37,7 @@ {{avatar $.Context .Issue.Poster 24}} - + {{template "shared/user/authorlink" .Issue.Poster}} {{.locale.Tr "repo.issues.commented_at" (.Issue.HashTag|Escape) $createdStr | Safe}} @@ -78,9 +78,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 "ctxData" $ "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 1a908f1aa..37931f287 100644 --- a/templates/repo/issue/view_content/add_reaction.tmpl +++ b/templates/repo/issue/view_content/add_reaction.tmpl @@ -1,13 +1,11 @@ {{if .ctxData.IsSigned}}
- + {{svg "octicon-smiley"}} -
-
{{.ctxData.locale.Tr "repo.pick_reaction"}}
-
+
{{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 5091201dd..0652198ad 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -87,9 +87,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 "ctxData" $ "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID) "Reactions" $reactions}} {{end}} @@ -471,9 +469,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 "ctxData" $ "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID) "Reactions" $reactions}} {{end}} @@ -595,9 +591,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 "ctxData" $ "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID) "Reactions" $reactions}} {{end}} diff --git a/templates/repo/issue/view_content/context_menu.tmpl b/templates/repo/issue/view_content/context_menu.tmpl index aa9d52b67..296668c9e 100644 --- a/templates/repo/issue/view_content/context_menu.tmpl +++ b/templates/repo/issue/view_content/context_menu.tmpl @@ -1,6 +1,6 @@ {{if .ctxData.IsSigned}}
- + {{svg "octicon-kebab-horizontal"}}
diff --git a/templates/repo/issue/view_content/reactions.tmpl b/templates/repo/issue/view_content/reactions.tmpl index 3c957d91c..02202783d 100644 --- a/templates/repo/issue/view_content/reactions.tmpl +++ b/templates/repo/issue/view_content/reactions.tmpl @@ -1,5 +1,10 @@ +
{{range $key, $value := .Reactions}} - + {{$hasReacted := $value.HasUser $.ctxData.SignedUserID}} + {{ReactionToEmoji $key}} {{len $value}} @@ -7,3 +12,4 @@ {{if AllowedReactions}} {{template "repo/issue/view_content/add_reaction" dict "ctxData" $.ctxData "ActionURL" .ActionURL}} {{end}} +
diff --git a/web_src/css/base.css b/web_src/css/base.css index a6576a4fe..21de3c826 100644 --- a/web_src/css/base.css +++ b/web_src/css/base.css @@ -165,8 +165,8 @@ --color-light: #00000006; --color-light-mimic-enabled: rgba(0, 0, 0, calc(6 / 255 * 222 / 255 / var(--opacity-disabled))); --color-light-border: #0000001d; - --color-hover: #0000000c; - --color-active: #00000014; + --color-hover: #00000014; + --color-active: #0000001e; --color-menu: #ffffff; --color-card: #ffffff; --color-markup-table-row: #00000008; @@ -184,7 +184,8 @@ --color-project-board-light-label: #a6aab5; --color-caret: var(--color-text-dark); --color-reaction-bg: #0000000a; - --color-reaction-active-bg: var(--color-primary-alpha-20); + --color-reaction-hover-bg: var(--color-primary-light-5); + --color-reaction-active-bg: var(--color-primary-light-6); --color-tooltip-text: #ffffff; --color-tooltip-bg: #000000f0; --color-header-bar: #ffffff; @@ -894,6 +895,11 @@ a.label, background: var(--color-hover); } +.ui.dropdown .menu > .item:active { + color: var(--color-text); + background: var(--color-active); +} + .ui.dropdown .menu .active.item { color: var(--color-text); background: var(--color-active); @@ -1059,10 +1065,15 @@ a.label, } .ui.pointing.dropdown > .menu:not(.hidden)::after { - background: var(--color-box-body); + background: var(--color-menu); box-shadow: -1px -1px 0 0 var(--color-secondary); } +.ui.pointing.upward.dropdown .menu::after, +.ui.top.pointing.upward.dropdown .menu::after { + box-shadow: 1px 1px 0 0 var(--color-secondary); +} + .ui.comments .comment .text { margin: 0; } diff --git a/web_src/css/repo.css b/web_src/css/repo.css index 069bf014b..daca98bb5 100644 --- a/web_src/css/repo.css +++ b/web_src/css/repo.css @@ -1085,7 +1085,7 @@ .repository.view.issue .comment-list .comment-code-cloud .segment.reactions .ui.label { border: 1px solid; - padding: 6px 10px !important; + padding: 5px 8px !important; margin: 0 2px; border-radius: var(--border-radius); border-color: var(--color-secondary-dark-1) !important; @@ -1096,6 +1096,11 @@ 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; } @@ -2133,7 +2138,7 @@ .repository .select-reaction.dropdown .menu { right: 0 !important; left: auto !important; - min-width: 15em; + min-width: 170px; } .repository .segment.reactions.dropdown .menu > .header, @@ -2144,25 +2149,15 @@ .repository .segment.reactions.dropdown .menu > .item, .repository .select-reaction.dropdown .menu > .item { float: left; - padding: 0.25rem !important; - margin: 0.25rem; - font-size: 1.5em; - width: 39px; - left: 13px; + margin: 4px; + font-size: 20px; + width: 34px; + height: 34px; + min-height: 0 !important; border-radius: 6px; - display: flex; - justify-content: center; + display: flex !important; align-items: center; -} - -.repository .segment.reactions.dropdown .menu > .item img.emoji, -.repository .select-reaction.dropdown .menu > .item img.emoji { - margin-right: 0; -} - -.repository .segment.reactions.dropdown .menu > .item:hover, -.repository .select-reaction.dropdown .menu > .item:hover { - background: var(--color-primary); + justify-content: center; } .repository .segment.reactions { @@ -2173,35 +2168,41 @@ 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: 10px 18px !important; + padding: 8px 16px !important; display: flex !important; align-items: center; border: 0; border-right: 1px solid; border-radius: 0; margin: 0; - font-size: 14px; + 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 { +.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.primary { - background-color: var(--color-reaction-active-bg) !important; - border-color: var(--color-secondary) !important; +.repository .segment.reactions .ui.label.basic:hover { + background-color: var(--color-reaction-hover-bg) !important; } .repository .segment.reactions .reaction-count { @@ -2211,6 +2212,9 @@ .repository .segment.reactions .select-reaction { display: flex; align-items: center; +} + +.repository .segment.reactions .select-reaction a { padding: 0 14px; } @@ -2469,7 +2473,7 @@ padding: 0.5rem 1rem; margin: 0 !important; position: relative; - color: var(--color-text-light-2); + color: var(--color-text); min-height: 41px; background-color: var(--color-box-header); display: flex; @@ -2516,14 +2520,6 @@ left: 7px; } -.comment-header a { - color: var(--color-text); -} - -.comment-header a:hover { - color: var(--color-primary); -} - .comment-header .actions a { margin-right: 0 !important; padding: 0.5rem !important; @@ -2534,6 +2530,11 @@ margin-left: 0.25rem; } +.content-history-menu .octicon-triangle-down { + position: relative; + top: 1.5px; +} + .comment-body { background: var(--color-box-body); border: none !important; diff --git a/web_src/css/themes/theme-arc-green.css b/web_src/css/themes/theme-arc-green.css index 5f2b32945..88dbe10d9 100644 --- a/web_src/css/themes/theme-arc-green.css +++ b/web_src/css/themes/theme-arc-green.css @@ -152,8 +152,8 @@ --color-light: #00000028; --color-light-mimic-enabled: rgba(0, 0, 0, calc(40 / 255 * 222 / 255 / var(--opacity-disabled))); --color-light-border: #ffffff28; - --color-hover: #ffffff10; - --color-active: #ffffff16; + --color-hover: #ffffff19; + --color-active: #ffffff24; --color-menu: #2e323e; --color-card: #2e323e; --color-markup-table-row: #ffffff06; @@ -170,7 +170,8 @@ --color-project-board-bg: var(--color-secondary-light-2); --color-caret: var(--color-text); /* should ideally be --color-text-dark, see #15651 */ --color-reaction-bg: #ffffff12; - --color-reaction-active-bg: var(--color-primary-alpha-40); + --color-reaction-hover-bg: var(--color-primary-light-4); + --color-reaction-active-bg: var(--color-primary-light-5); --color-tooltip-text: #ffffff; --color-tooltip-bg: #000000f0; --color-header-bar: #2e323e; diff --git a/web_src/js/features/comp/ReactionSelector.js b/web_src/js/features/comp/ReactionSelector.js index 33ceb73c7..336634a58 100644 --- a/web_src/js/features/comp/ReactionSelector.js +++ b/web_src/js/features/comp/ReactionSelector.js @@ -1,53 +1,46 @@ import $ from 'jquery'; -import {createTippy} from '../../modules/tippy.js'; const {csrfToken} = window.config; -export function initCompReactionSelector(parent) { - let selector = 'a.label'; - if (!parent) { - parent = $(document); - selector = `.reactions ${selector}`; - } - - for (const el of parent[0].querySelectorAll(selector)) { - createTippy(el, {placement: 'bottom-start', content: el.getAttribute('data-title')}); - } - - parent.find(`.select-reaction > .menu > .item, ${selector}`).on('click', function (e) { +export function initCompReactionSelector($parent) { + $parent.find(`.select-reaction .item.reaction, .comment-reaction-button`).on('click', async function (e) { e.preventDefault(); if ($(this).hasClass('disabled')) return; - const actionURL = $(this).hasClass('item') ? $(this).closest('.select-reaction').data('action-url') : $(this).data('action-url'); - const url = `${actionURL}/${$(this).hasClass('primary') ? 'unreact' : 'react'}`; - $.ajax({ - type: 'POST', - url, - data: { + const actionUrl = $(this).closest('[data-action-url]').attr('data-action-url'); + const reactionContent = $(this).attr('data-reaction-content'); + const hasReacted = $(this).closest('.ui.segment.reactions').find(`a[data-reaction-content="${reactionContent}"]`).attr('data-has-reacted') === 'true'; + + const res = await fetch(`${actionUrl}/${hasReacted ? 'unreact' : 'react'}`, { + method: 'POST', + headers: { + 'content-type': 'application/x-www-form-urlencoded', + }, + body: new URLSearchParams({ _csrf: csrfToken, - content: $(this).attr('data-reaction-content'), - } - }).done((resp) => { - if (resp && (resp.html || resp.empty)) { - const content = $(this).closest('.content'); - let react = content.find('.segment.reactions'); - if ((!resp.empty || resp.html === '') && react.length > 0) { - react.remove(); - } - if (!resp.empty) { - react = $('
'); - const attachments = content.find('.segment.bottom:first'); - if (attachments.length > 0) { - react.insertBefore(attachments); - } else { - react.appendTo(content); - } - react.html(resp.html); - react.find('.dropdown').dropdown(); - initCompReactionSelector(react); - } - } + content: reactionContent, + }), }); + + 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-code.js b/web_src/js/features/repo-code.js index 73eccaf19..26d8e2247 100644 --- a/web_src/js/features/repo-code.js +++ b/web_src/js/features/repo-code.js @@ -190,8 +190,6 @@ export function initRepoCodeView() { currentTarget.closest('tr').outerHTML = blob; }); $(document).on('click', '.copy-line-permalink', async (e) => { - const success = await clippie(toAbsoluteUrl(e.currentTarget.getAttribute('data-url'))); - if (!success) return; - document.querySelector('.code-line-button')?._tippy?.hide(); + await clippie(toAbsoluteUrl(e.currentTarget.getAttribute('data-url'))); }); } diff --git a/web_src/js/features/repo-issue-content.js b/web_src/js/features/repo-issue-content.js index 87295573e..792a11c5a 100644 --- a/web_src/js/features/repo-issue-content.js +++ b/web_src/js/features/repo-issue-content.js @@ -76,7 +76,7 @@ function showContentHistoryMenu(issueBaseUrl, $item, commentId) { const $headerLeft = $item.find('.comment-header-left'); const menuHtml = `
- • ${i18nTextEdited}${svg('octicon-triangle-down', 14, 'dropdown icon gt-ml-1 gt-mt-1')} + • ${i18nTextEdited}${svg('octicon-triangle-down', 14, 'dropdown icon gt-ml-1 gt-mt-1')}
`; diff --git a/web_src/js/features/repo-legacy.js b/web_src/js/features/repo-legacy.js index 2804844d8..f23ff4547 100644 --- a/web_src/js/features/repo-legacy.js +++ b/web_src/js/features/repo-legacy.js @@ -544,7 +544,7 @@ export function initRepository() { initRepoIssueDependencyDelete(); initRepoIssueCodeCommentCancel(); initRepoPullRequestUpdate(); - initCompReactionSelector(); + initCompReactionSelector($(document)); initRepoPullRequestMergeForm(); } diff --git a/web_src/js/modules/tippy.js b/web_src/js/modules/tippy.js index df2143b0e..d9f496fb8 100644 --- a/web_src/js/modules/tippy.js +++ b/web_src/js/modules/tippy.js @@ -58,10 +58,17 @@ function attachTooltip(target, content = null) { content = content ?? target.getAttribute('data-tooltip-content'); if (!content) return null; + // when element has a clipboard target, we update the tooltip after copy + // in which case it is undesirable to automatically hide it on click as + // it would momentarily flash the tooltip out and in. + const hasClipboardTarget = target.hasAttribute('data-clipboard-target'); + const hideOnClick = !hasClipboardTarget; + const props = { content, delay: 100, role: 'tooltip', + hideOnClick, placement: target.getAttribute('data-tooltip-placement') || 'top-start', ...(target.getAttribute('data-tooltip-interactive') === 'true' ? {interactive: true, aria: {content: 'describedby', expanded: false}} : {}), };