Make issue meta dropdown support Enter, confirm before reloading (#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:

<details>

![image](https://user-images.githubusercontent.com/2114189/220538288-e2da8459-6a4e-43cb-8596-74057f8a03a2.png)

</details>

---------

Co-authored-by: Brecht Van Lommel <brecht@blender.org>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

diff --git a/templates/repo/issue/view_content/add_reaction.tmpl b/templates/repo/issue/view_content/add_reaction.tmpl
index 9bd9022d28abf3c941772fdbcef616260db993fb..bfa8a7e12234901196d4f6a764dce794c33b4646 100644 (file)
--- a/templates/repo/issue/view_content/add_reaction.tmpl
+++ b/templates/repo/issue/view_content/add_reaction.tmpl
@@ -7,7 +7,7 @@
                <div class="header">{{.ctx.locale.Tr "repo.pick_reaction"}}</div>
                <div class="divider"></div>
                {{range $value := AllowedReactions}}
-                       <div class="item reaction tooltip" data-content="{{$value}}">{{ReactionToEmoji $value}}</div>
+                       <a class="item reaction tooltip" data-content="{{$value}}">{{ReactionToEmoji $value}}</a>
                {{end}}
        </div>
 </div>

diff --git a/templates/repo/issue/view_content/context_menu.tmpl b/templates/repo/issue/view_content/context_menu.tmpl
index b96fd86af5d00f91ba21ad86a3cf7a6cfe1be974..b4b9403b2af74a281844c78e8d8ca47905da16d5 100644 (file)
--- 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}}
-               <div class="item context" data-clipboard-text-type="url" data-clipboard-text="{{AppSubUrl}}{{$referenceUrl}}">{{.ctx.locale.Tr "repo.issues.context.copy_link"}}</div>
-               <div class="item context quote-reply {{if .diff}}quote-reply-diff{{end}}" data-target="{{.item.HashTag}}-raw">{{.ctx.locale.Tr "repo.issues.context.quote_reply"}}</div>
+               <a class="item context" data-clipboard-text-type="url" data-clipboard-text="{{AppSubUrl}}{{$referenceUrl}}">{{.ctx.locale.Tr "repo.issues.context.copy_link"}}</a>
+               <a class="item context quote-reply {{if .diff}}quote-reply-diff{{end}}" data-target="{{.item.HashTag}}-raw">{{.ctx.locale.Tr "repo.issues.context.quote_reply"}}</a>
                {{if not .ctx.UnitIssuesGlobalDisabled}}
-                       <div class="item context reference-issue" data-target="{{.item.HashTag}}-raw" data-modal="#reference-issue-modal" data-poster="{{.item.Poster.GetDisplayName}}" data-poster-username="{{.item.Poster.Name}}" data-reference="{{$referenceUrl}}">{{.ctx.locale.Tr "repo.issues.context.reference_issue"}}</div>
+                       <a class="item context reference-issue" data-target="{{.item.HashTag}}-raw" data-modal="#reference-issue-modal" data-poster="{{.item.Poster.GetDisplayName}}" data-poster-username="{{.item.Poster.Name}}" data-reference="{{$referenceUrl}}">{{.ctx.locale.Tr "repo.issues.context.reference_issue"}}</a>
                {{end}}
                {{if or .ctx.Permission.IsAdmin .IsCommentPoster .ctx.HasIssuesOrPullsWritePermission}}
                        <div class="divider"></div>
-                       <div class="item context edit-content">{{.ctx.locale.Tr "repo.issues.context.edit"}}</div>
+                       <a class="item context edit-content">{{.ctx.locale.Tr "repo.issues.context.edit"}}</a>
                        {{if .delete}}
-                               <div class="item context delete-comment" data-comment-id={{.item.HashTag}} data-url="{{.ctx.RepoLink}}/comments/{{.item.ID}}/delete" data-locale="{{.ctx.locale.Tr "repo.issues.delete_comment_confirm"}}">{{.ctx.locale.Tr "repo.issues.context.delete"}}</div>
+                               <a class="item context delete-comment" data-comment-id={{.item.HashTag}} data-url="{{.ctx.RepoLink}}/comments/{{.item.ID}}/delete" data-locale="{{.ctx.locale.Tr "repo.issues.delete_comment_confirm"}}">{{.ctx.locale.Tr "repo.issues.context.delete"}}</a>
                        {{end}}
                {{end}}
        </div>

diff --git a/templates/repo/issue/view_content/sidebar.tmpl b/templates/repo/issue/view_content/sidebar.tmpl
index 2b24825b5f7bdd20e1acfce75fb0aa279c4f5420..d9c506243705f1d18d73d49924ae27e0b6c216d8 100644 (file)
--- a/templates/repo/issue/view_content/sidebar.tmpl
+++ b/templates/repo/issue/view_content/sidebar.tmpl
@@ -121,7 +121,7 @@
                                                <input type="text" placeholder="{{.locale.Tr "repo.issues.filter_labels"}}">
                                        </div>
                                {{end}}
-                               <div class="no-select item">{{.locale.Tr "repo.issues.new.clear_labels"}}</div>
+                               <a class="no-select item" href="#">{{.locale.Tr "repo.issues.new.clear_labels"}}</a>
                                {{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 a5ac84e4464c52968a6918c45d6f05fcb82e7c2c..373d667c5f73c98d7ed82fd0184640cd15fe30ed 100644 (file)
--- 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 8178ed6547f09d2c96c8ab81ceedb548e7d33285..a9229c0d1e6f565ba71c36fcd1598017bc17c670 100644 (file)
--- 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('');