Skip to content

Commit 5ddf67a

Browse files
yardenshohamwxiaoguangbrechtvllunny
authored
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: <details> ![image](https://user-images.githubusercontent.com/2114189/220538288-e2da8459-6a4e-43cb-8596-74057f8a03a2.png) </details> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: Brecht Van Lommel <brecht@blender.org> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
1 parent 4d3e2b2 commit 5ddf67a

File tree

5 files changed

+46
-20
lines changed

5 files changed

+46
-20
lines changed

Diff for: ‎templates/repo/issue/view_content/add_reaction.tmpl

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
<div class="header">{{.ctx.locale.Tr "repo.pick_reaction"}}</div>
88
<div class="divider"></div>
99
{{range $value := AllowedReactions}}
10-
<div class="item reaction tooltip" data-content="{{$value}}">{{ReactionToEmoji $value}}</div>
10+
<a class="item reaction tooltip" data-content="{{$value}}">{{ReactionToEmoji $value}}</a>
1111
{{end}}
1212
</div>
1313
</div>

Diff for: ‎templates/repo/issue/view_content/context_menu.tmpl

+5-5
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,16 @@
1010
{{else}}
1111
{{$referenceUrl = Printf "%s/files#%s" .ctx.Issue.Link .item.HashTag}}
1212
{{end}}
13-
<div class="item context" data-clipboard-text-type="url" data-clipboard-text="{{AppSubUrl}}{{$referenceUrl}}">{{.ctx.locale.Tr "repo.issues.context.copy_link"}}</div>
14-
<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>
13+
<a class="item context" data-clipboard-text-type="url" data-clipboard-text="{{AppSubUrl}}{{$referenceUrl}}">{{.ctx.locale.Tr "repo.issues.context.copy_link"}}</a>
14+
<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>
1515
{{if not .ctx.UnitIssuesGlobalDisabled}}
16-
<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>
16+
<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>
1717
{{end}}
1818
{{if or .ctx.Permission.IsAdmin .IsCommentPoster .ctx.HasIssuesOrPullsWritePermission}}
1919
<div class="divider"></div>
20-
<div class="item context edit-content">{{.ctx.locale.Tr "repo.issues.context.edit"}}</div>
20+
<a class="item context edit-content">{{.ctx.locale.Tr "repo.issues.context.edit"}}</a>
2121
{{if .delete}}
22-
<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>
22+
<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>
2323
{{end}}
2424
{{end}}
2525
</div>

Diff for: ‎templates/repo/issue/view_content/sidebar.tmpl

+1-1
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@
121121
<input type="text" placeholder="{{.locale.Tr "repo.issues.filter_labels"}}">
122122
</div>
123123
{{end}}
124-
<div class="no-select item">{{.locale.Tr "repo.issues.new.clear_labels"}}</div>
124+
<a class="no-select item" href="#">{{.locale.Tr "repo.issues.new.clear_labels"}}</a>
125125
{{if or .Labels .OrgLabels}}
126126
{{$previousExclusiveScope := "_no_scope"}}
127127
{{range .Labels}}

Diff for: ‎web_src/js/features/aria.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,8 @@ function attachOneDropdownAria($dropdown) {
8181
$dropdown.on('keydown', (e) => {
8282
// here it must use keydown event before dropdown's keyup handler, otherwise there is no Enter event in our keyup handler
8383
if (e.key === 'Enter') {
84-
const $item = $dropdown.dropdown('get item', $dropdown.dropdown('get value'));
84+
let $item = $dropdown.dropdown('get item', $dropdown.dropdown('get value'));
85+
if (!$item) $item = $menu.find('> .item.selected'); // when dropdown filters items by input, there is no "value", so query the "selected" item
8586
// if the selected item is clickable, then trigger the click event. in the future there could be a special CSS class for it.
8687
if ($item && $item.is('a')) $item[0].click();
8788
}

Diff for: ‎web_src/js/features/repo-legacy.js

+37-12
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,26 @@ import {hideElem, showElem} from '../utils/dom.js';
2929

3030
const {csrfToken} = window.config;
3131

32+
// if there are draft comments (more than 20 chars), confirm before reloading, to avoid losing comments
33+
function reloadConfirmDraftComment() {
34+
const commentTextareas = [
35+
document.querySelector('.edit-content-zone:not(.gt-hidden) textarea'),
36+
document.querySelector('.edit_area'),
37+
];
38+
for (const textarea of commentTextareas) {
39+
// Most users won't feel too sad if they lose a comment with 10 or 20 chars, they can re-type these in seconds.
40+
// But if they have typed more (like 50) chars and the comment is lost, they will be very unhappy.
41+
if (textarea && textarea.value.trim().length > 20) {
42+
textarea.parentElement.scrollIntoView();
43+
if (!window.confirm('Page will be reloaded, but there are draft comments. Continuing to reload will discard the comments. Continue?')) {
44+
return;
45+
}
46+
break;
47+
}
48+
}
49+
window.location.reload();
50+
}
51+
3252
export function initRepoCommentForm() {
3353
const $commentForm = $('.comment.form');
3454
if ($commentForm.length === 0) {
@@ -86,22 +106,27 @@ export function initRepoCommentForm() {
86106
let hasUpdateAction = $listMenu.data('action') === 'update';
87107
const items = {};
88108

89-
$(`.${selector}`).dropdown('setting', 'onHide', () => {
90-
hasUpdateAction = $listMenu.data('action') === 'update'; // Update the var
91-
if (hasUpdateAction) {
92-
// TODO: Add batch functionality and make this 1 network request.
93-
(async function() {
94-
for (const [elementId, item] of Object.entries(items)) {
109+
$(`.${selector}`).dropdown({
110+
'action': 'nothing', // do not hide the menu if user presses Enter
111+
fullTextSearch: 'exact',
112+
async onHide() {
113+
hasUpdateAction = $listMenu.data('action') === 'update'; // Update the var
114+
if (hasUpdateAction) {
115+
// TODO: Add batch functionality and make this 1 network request.
116+
const itemEntries = Object.entries(items);
117+
for (const [elementId, item] of itemEntries) {
95118
await updateIssuesMeta(
96119
item['update-url'],
97120
item.action,
98121
item['issue-id'],
99122
elementId,
100123
);
101124
}
102-
window.location.reload();
103-
})();
104-
}
125+
if (itemEntries.length) {
126+
reloadConfirmDraftComment();
127+
}
128+
}
129+
},
105130
});
106131

107132
$listMenu.find('.item:not(.no-select)').on('click', function (e) {
@@ -196,7 +221,7 @@ export function initRepoCommentForm() {
196221
'clear',
197222
$listMenu.data('issue-id'),
198223
'',
199-
).then(() => window.location.reload());
224+
).then(reloadConfirmDraftComment);
200225
}
201226

202227
$(this).parent().find('.item').each(function () {
@@ -239,7 +264,7 @@ export function initRepoCommentForm() {
239264
'',
240265
$menu.data('issue-id'),
241266
$(this).data('id'),
242-
).then(() => window.location.reload());
267+
).then(reloadConfirmDraftComment);
243268
}
244269

245270
let icon = '';
@@ -272,7 +297,7 @@ export function initRepoCommentForm() {
272297
'',
273298
$menu.data('issue-id'),
274299
$(this).data('id'),
275-
).then(() => window.location.reload());
300+
).then(reloadConfirmDraftComment);
276301
}
277302

278303
$list.find('.selected').html('');

0 commit comments

Comments
 (0)