From 5ddf67a9c2325282a2121672d7d7ea77789e2cf8 Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Fri, 24 Feb 2023 11:40:36 +0200 Subject: [PATCH] Make issue meta dropdown support Enter, confirm before reloading (#23014) (#23102) Backport #23014 As the title. Label/assignee share the same code. * Close #22607 * Close #20727 Also: * partially fix for #21742, now the comment reaction and menu work with keyboard. * partially fix for #17705, in most cases the comment won't be lost. * partially fix for #21539 * partially fix for #20347 * partially fix for #7329 ### The `Enter` support Before, if user presses Enter, the dropdown just disappears and nothing happens or the window reloads. After, Enter can be used to select/deselect labels, and press Esc to hide the dropdown to update the labels (still no way to cancel .... maybe you can do a Cmd+R or F5 to refresh the window to discard the changes .....) This is only a quick patch, the UX is still not perfect, but it's much better than before. ### The `confirm` before reloading And more fixes for the `reload` problem, the new behaviors: * If nothing changes (just show/hide the dropdown), then the page won't be reloaded. * If there are draft comments, show a confirm dialog before reloading, to avoid losing comments. That's the best effect can be done at the moment, unless completely refactor these dropdown related code. Screenshot of the confirm dialog:
![image](https://user-images.githubusercontent.com/2114189/220538288-e2da8459-6a4e-43cb-8596-74057f8a03a2.png)
Co-authored-by: wxiaoguang Co-authored-by: Brecht Van Lommel Co-authored-by: Lunny Xiao --- .../repo/issue/view_content/add_reaction.tmpl | 2 +- .../repo/issue/view_content/context_menu.tmpl | 10 ++-- .../repo/issue/view_content/sidebar.tmpl | 2 +- web_src/js/features/aria.js | 3 +- web_src/js/features/repo-legacy.js | 49 ++++++++++++++----- 5 files changed, 46 insertions(+), 20 deletions(-) diff --git a/templates/repo/issue/view_content/add_reaction.tmpl b/templates/repo/issue/view_content/add_reaction.tmpl index 9bd9022d28ab..bfa8a7e12234 100644 --- a/templates/repo/issue/view_content/add_reaction.tmpl +++ b/templates/repo/issue/view_content/add_reaction.tmpl @@ -7,7 +7,7 @@
{{.ctx.locale.Tr "repo.pick_reaction"}}
{{range $value := AllowedReactions}} -
{{ReactionToEmoji $value}}
+ {{ReactionToEmoji $value}} {{end}} diff --git a/templates/repo/issue/view_content/context_menu.tmpl b/templates/repo/issue/view_content/context_menu.tmpl index b96fd86af5d0..b4b9403b2af7 100644 --- a/templates/repo/issue/view_content/context_menu.tmpl +++ b/templates/repo/issue/view_content/context_menu.tmpl @@ -10,16 +10,16 @@ {{else}} {{$referenceUrl = Printf "%s/files#%s" .ctx.Issue.Link .item.HashTag}} {{end}} -
{{.ctx.locale.Tr "repo.issues.context.copy_link"}}
-
{{.ctx.locale.Tr "repo.issues.context.quote_reply"}}
+ {{.ctx.locale.Tr "repo.issues.context.copy_link"}} + {{.ctx.locale.Tr "repo.issues.context.quote_reply"}} {{if not .ctx.UnitIssuesGlobalDisabled}} -
{{.ctx.locale.Tr "repo.issues.context.reference_issue"}}
+ {{.ctx.locale.Tr "repo.issues.context.reference_issue"}} {{end}} {{if or .ctx.Permission.IsAdmin .IsCommentPoster .ctx.HasIssuesOrPullsWritePermission}}
-
{{.ctx.locale.Tr "repo.issues.context.edit"}}
+ {{.ctx.locale.Tr "repo.issues.context.edit"}} {{if .delete}} -
{{.ctx.locale.Tr "repo.issues.context.delete"}}
+ {{.ctx.locale.Tr "repo.issues.context.delete"}} {{end}} {{end}} diff --git a/templates/repo/issue/view_content/sidebar.tmpl b/templates/repo/issue/view_content/sidebar.tmpl index 2b24825b5f7b..d9c506243705 100644 --- a/templates/repo/issue/view_content/sidebar.tmpl +++ b/templates/repo/issue/view_content/sidebar.tmpl @@ -121,7 +121,7 @@ {{end}} -
{{.locale.Tr "repo.issues.new.clear_labels"}}
+ {{.locale.Tr "repo.issues.new.clear_labels"}} {{if or .Labels .OrgLabels}} {{$previousExclusiveScope := "_no_scope"}} {{range .Labels}} diff --git a/web_src/js/features/aria.js b/web_src/js/features/aria.js index a5ac84e4464c..373d667c5f73 100644 --- a/web_src/js/features/aria.js +++ b/web_src/js/features/aria.js @@ -81,7 +81,8 @@ function attachOneDropdownAria($dropdown) { $dropdown.on('keydown', (e) => { // here it must use keydown event before dropdown's keyup handler, otherwise there is no Enter event in our keyup handler if (e.key === 'Enter') { - const $item = $dropdown.dropdown('get item', $dropdown.dropdown('get value')); + let $item = $dropdown.dropdown('get item', $dropdown.dropdown('get value')); + if (!$item) $item = $menu.find('> .item.selected'); // when dropdown filters items by input, there is no "value", so query the "selected" item // if the selected item is clickable, then trigger the click event. in the future there could be a special CSS class for it. if ($item && $item.is('a')) $item[0].click(); } diff --git a/web_src/js/features/repo-legacy.js b/web_src/js/features/repo-legacy.js index 8178ed6547f0..a9229c0d1e6f 100644 --- a/web_src/js/features/repo-legacy.js +++ b/web_src/js/features/repo-legacy.js @@ -29,6 +29,26 @@ import {hideElem, showElem} from '../utils/dom.js'; const {csrfToken} = window.config; +// if there are draft comments (more than 20 chars), confirm before reloading, to avoid losing comments +function reloadConfirmDraftComment() { + const commentTextareas = [ + document.querySelector('.edit-content-zone:not(.gt-hidden) textarea'), + document.querySelector('.edit_area'), + ]; + for (const textarea of commentTextareas) { + // Most users won't feel too sad if they lose a comment with 10 or 20 chars, they can re-type these in seconds. + // But if they have typed more (like 50) chars and the comment is lost, they will be very unhappy. + if (textarea && textarea.value.trim().length > 20) { + textarea.parentElement.scrollIntoView(); + if (!window.confirm('Page will be reloaded, but there are draft comments. Continuing to reload will discard the comments. Continue?')) { + return; + } + break; + } + } + window.location.reload(); +} + export function initRepoCommentForm() { const $commentForm = $('.comment.form'); if ($commentForm.length === 0) { @@ -86,12 +106,15 @@ export function initRepoCommentForm() { let hasUpdateAction = $listMenu.data('action') === 'update'; const items = {}; - $(`.${selector}`).dropdown('setting', 'onHide', () => { - hasUpdateAction = $listMenu.data('action') === 'update'; // Update the var - if (hasUpdateAction) { - // TODO: Add batch functionality and make this 1 network request. - (async function() { - for (const [elementId, item] of Object.entries(items)) { + $(`.${selector}`).dropdown({ + 'action': 'nothing', // do not hide the menu if user presses Enter + fullTextSearch: 'exact', + async onHide() { + hasUpdateAction = $listMenu.data('action') === 'update'; // Update the var + if (hasUpdateAction) { + // TODO: Add batch functionality and make this 1 network request. + const itemEntries = Object.entries(items); + for (const [elementId, item] of itemEntries) { await updateIssuesMeta( item['update-url'], item.action, @@ -99,9 +122,11 @@ export function initRepoCommentForm() { elementId, ); } - window.location.reload(); - })(); - } + if (itemEntries.length) { + reloadConfirmDraftComment(); + } + } + }, }); $listMenu.find('.item:not(.no-select)').on('click', function (e) { @@ -196,7 +221,7 @@ export function initRepoCommentForm() { 'clear', $listMenu.data('issue-id'), '', - ).then(() => window.location.reload()); + ).then(reloadConfirmDraftComment); } $(this).parent().find('.item').each(function () { @@ -239,7 +264,7 @@ export function initRepoCommentForm() { '', $menu.data('issue-id'), $(this).data('id'), - ).then(() => window.location.reload()); + ).then(reloadConfirmDraftComment); } let icon = ''; @@ -272,7 +297,7 @@ export function initRepoCommentForm() { '', $menu.data('issue-id'), $(this).data('id'), - ).then(() => window.location.reload()); + ).then(reloadConfirmDraftComment); } $list.find('.selected').html('');