Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Batch delete issue and improve tippy opts #25253

Merged
merged 11 commits into from
Jun 19, 2023
4 changes: 4 additions & 0 deletions modules/context/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,10 @@ func (b *Base) JSONRedirect(redirect string) {
b.JSON(http.StatusOK, map[string]any{"redirect": redirect})
}

func (b *Base) JSONOK() {
b.JSON(http.StatusOK, map[string]any{"ok": true}) // this is only a dummy response, frontend seldom uses it
}

func (b *Base) JSONError(msg string) {
b.JSON(http.StatusBadRequest, map[string]any{"errorMessage": msg})
}
Expand Down
2 changes: 2 additions & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ show_timestamps = Show timestamps
show_log_seconds = Show seconds
show_full_screen = Show full screen

confirm_delete_selected = Confirm to delete all selected items?

[aria]
navbar = Navigation Bar
footer = Footer
Expand Down
18 changes: 15 additions & 3 deletions routers/web/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -2705,6 +2705,20 @@ func ListIssues(ctx *context.Context) {
ctx.JSON(http.StatusOK, convert.ToAPIIssueList(ctx, issues))
}

func BatchDeleteIssues(ctx *context.Context) {
issues := getActionIssues(ctx)
if ctx.Written() {
return
}
for _, issue := range issues {
if err := issue_service.DeleteIssue(ctx, ctx.Doer, ctx.Repo.GitRepo, issue); err != nil {
ctx.ServerError("DeleteIssue", err)
return
}
}
ctx.JSONOK()
}

// UpdateIssueStatus change issue's status
func UpdateIssueStatus(ctx *context.Context) {
issues := getActionIssues(ctx)
Expand Down Expand Up @@ -2740,9 +2754,7 @@ func UpdateIssueStatus(ctx *context.Context) {
}
}
}
ctx.JSON(http.StatusOK, map[string]interface{}{
"ok": true,
})
ctx.JSONOK()
}

// NewComment create a comment for issue
Expand Down
1 change: 1 addition & 0 deletions routers/web/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -1024,6 +1024,7 @@ func registerRoutes(m *web.Route) {
m.Post("/request_review", reqRepoIssuesOrPullsReader, repo.UpdatePullReviewRequest)
m.Post("/dismiss_review", reqRepoAdmin, web.Bind(forms.DismissReviewForm{}), repo.DismissReview)
m.Post("/status", reqRepoIssuesOrPullsWriter, repo.UpdateIssueStatus)
m.Post("/delete", reqRepoAdmin, repo.BatchDeleteIssues)
m.Post("/resolve_conversation", reqRepoIssuesOrPullsReader, repo.UpdateResolveConversation)
m.Post("/attachments", repo.UploadIssueAttachment)
m.Post("/attachments/remove", repo.DeleteAttachment)
Expand Down
4 changes: 3 additions & 1 deletion templates/devtest/fetch-action.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
It might be renamed to "link-fetch-action" to match the "form-fetch-action".
</div>
<div>
<button class="link-action" data-url="fetch-action-test?k=1">test</button>
<button class="link-action" data-url="fetch-action-test?k=1">test action</button>
<button class="link-action" data-url="fetch-action-test?k=1" data-modal-confirm="confirm?">test with confirm</button>
<button class="ui red button link-action" data-url="fetch-action-test?k=1" data-modal-confirm="confirm?">test with risky confirm</button>
</div>
</div>
<div>
Expand Down
10 changes: 8 additions & 2 deletions templates/repo/issue/list.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -282,9 +282,15 @@
{{if not .Repository.IsArchived}}
<!-- Action Button -->
{{if .IsShowClosed}}
<button class="ui green active basic button issue-action gt-ml-auto" data-action="open" data-url="{{$.RepoLink}}/issues/status">{{.locale.Tr "repo.issues.action_open"}}</button>
<button class="ui green basic button issue-action gt-ml-auto" data-action="open" data-url="{{$.RepoLink}}/issues/status">{{.locale.Tr "repo.issues.action_open"}}</button>
{{else}}
<button class="ui red active basic button issue-action gt-ml-auto" data-action="close" data-url="{{$.RepoLink}}/issues/status">{{.locale.Tr "repo.issues.action_close"}}</button>
<button class="ui red basic button issue-action gt-ml-auto" data-action="close" data-url="{{$.RepoLink}}/issues/status">{{.locale.Tr "repo.issues.action_close"}}</button>
{{end}}
{{if $.IsRepoAdmin}}
<button class="ui red button issue-action gt-ml-auto"
data-action="delete" data-url="{{$.RepoLink}}/issues/delete"
data-action-delete-confirm="{{.locale.Tr "confirm_delete_selected"}}"
>{{.locale.Tr "repo.issues.delete"}}</button>
{{end}}
<!-- Labels -->
<div class="ui {{if not .Labels}}disabled{{end}} dropdown jump item">
Expand Down
32 changes: 8 additions & 24 deletions web_src/js/features/common-global.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {svg} from '../svg.js';
import {hideElem, showElem, toggleElem} from '../utils/dom.js';
import {htmlEscape} from 'escape-goat';
import {createTippy} from '../modules/tippy.js';
import {confirmModal} from './comp/ConfirmModal.js';

const {appUrl, appSubUrl, csrfToken, i18n} = window.config;

Expand Down Expand Up @@ -264,7 +265,7 @@ export function initGlobalDropzone() {
}
}

function linkAction(e) {
async function linkAction(e) {
e.preventDefault();

// A "link-action" can post AJAX request to its "data-url"
Expand All @@ -291,33 +292,16 @@ function linkAction(e) {
});
};

const modalConfirmHtml = htmlEscape($this.attr('data-modal-confirm') || '');
if (!modalConfirmHtml) {
const modalConfirmContent = htmlEscape($this.attr('data-modal-confirm') || '');
if (!modalConfirmContent) {
doRequest();
return;
}

const okButtonColor = $this.hasClass('red') || $this.hasClass('yellow') || $this.hasClass('orange') || $this.hasClass('negative') ? 'orange' : 'green';

const $modal = $(`
<div class="ui g-modal-confirm modal">
<div class="content">${modalConfirmHtml}</div>
<div class="actions">
<button class="ui basic cancel button">${svg('octicon-x')} ${i18n.modal_cancel}</button>
<button class="ui ${okButtonColor} ok button">${svg('octicon-check')} ${i18n.modal_confirm}</button>
</div>
</div>
`);

$modal.appendTo(document.body);
$modal.modal({
onApprove() {
doRequest();
},
onHidden() {
$modal.remove();
},
}).modal('show');
const isRisky = $this.hasClass('red') || $this.hasClass('yellow') || $this.hasClass('orange') || $this.hasClass('negative');
if (await confirmModal({content: modalConfirmContent, buttonColor: isRisky ? 'orange' : 'green'})) {
doRequest();
}
}

export function initGlobalLinkActions() {
Expand Down
30 changes: 30 additions & 0 deletions web_src/js/features/comp/ConfirmModal.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import $ from 'jquery';
import {svg} from '../../svg.js';
import {htmlEscape} from 'escape-goat';

const {i18n} = window.config;

export async function confirmModal(opts = {content: '', buttonColor: 'green'}) {
return new Promise((resolve) => {
const $modal = $(`
<div class="ui g-modal-confirm modal">
<div class="content">${htmlEscape(opts.content)}</div>
<div class="actions">
<button class="ui basic cancel button">${svg('octicon-x')} ${i18n.modal_cancel}</button>
wxiaoguang marked this conversation as resolved.
Show resolved Hide resolved
<button class="ui ${opts.buttonColor || 'green'} ok button">${svg('octicon-check')} ${i18n.modal_confirm}</button>
wxiaoguang marked this conversation as resolved.
Show resolved Hide resolved
</div>
</div>
`);

$modal.appendTo(document.body);
$modal.modal({
onApprove() {
resolve(true);
},
onHidden() {
$modal.remove();
resolve(false);
},
}).modal('show');
});
}
28 changes: 23 additions & 5 deletions web_src/js/features/repo-issue-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {updateIssuesMeta} from './repo-issue.js';
import {toggleElem} from '../utils/dom.js';
import {htmlEscape} from 'escape-goat';
import {Sortable} from 'sortablejs';
import {confirmModal} from './comp/ConfirmModal.js';

function initRepoIssueListCheckboxes() {
const $issueSelectAll = $('.issue-checkbox-all');
Expand Down Expand Up @@ -36,19 +37,36 @@ function initRepoIssueListCheckboxes() {

$('.issue-action').on('click', async function (e) {
e.preventDefault();

const url = this.getAttribute('data-url');
let action = this.getAttribute('data-action');
let elementId = this.getAttribute('data-element-id');
const url = this.getAttribute('data-url');
const issueIDs = $('.issue-checkbox:checked').map((_, el) => {
return el.getAttribute('data-issue-id');
}).get().join(',');
if (elementId === '0' && url.slice(-9) === '/assignee') {
let issueIDs = [];
for (const el of document.querySelectorAll('.issue-checkbox:checked')) {
issueIDs.push(el.getAttribute('data-issue-id'));
}
issueIDs = issueIDs.join(',');
if (!issueIDs) return;

// for assignee
if (elementId === '0' && url.endsWith('/assignee')) {
elementId = '';
action = 'clear';
}

// for toggle
if (action === 'toggle' && e.altKey) {
action = 'toggle-alt';
}

// for delete
if (action === 'delete') {
const confirmText = e.target.getAttribute('data-action-delete-confirm');
if (!await confirmModal({content: confirmText, buttonColor: 'orange'})) {
return;
}
}

updateIssuesMeta(
url,
action,
Expand Down
22 changes: 10 additions & 12 deletions web_src/js/modules/tippy.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@ import tippy from 'tippy.js';
const visibleInstances = new Set();

export function createTippy(target, opts = {}) {
const {role, content, onHide: optsOnHide, onDestroy: optsOnDestroy, onShow: optOnShow} = opts;
delete opts.onHide;
delete opts.onDestroy;
delete opts.onShow;

// the callback functions should be destructured from opts,
// because we should use our own wrapper functions to handle them, do not let the user override them
const {onHide, onShow, onDestroy, ...other} = opts;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would also destructure role and content here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can even shorten it by placing the destructure in the function definition:

export function createTippy(target, {onHide, onShow, onDestroy, role, content, ...other} = {})

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would also destructure role and content here.

IMO the code would be more complex for using "role" later. Could you try to improve it by your idea?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

export function createTippy(target, {onHide, onShow, onDestroy, role, content, ...other} = {})

Well, personally I dislike such syntax. But if you prefer, no block from my side.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely prefer that, it's most readable to me. Don't have time right now, but I can do a few modifications later.

const instance = tippy(target, {
appendTo: document.body,
animation: false,
Expand All @@ -18,11 +16,11 @@ export function createTippy(target, opts = {}) {
maxWidth: 500, // increase over default 350px
onHide: (instance) => {
visibleInstances.delete(instance);
return optsOnHide?.(instance);
return onHide?.(instance);
},
onDestroy: (instance) => {
visibleInstances.delete(instance);
return optsOnDestroy?.(instance);
return onDestroy?.(instance);
},
onShow: (instance) => {
// hide other tooltip instances so only one tooltip shows at a time
Expand All @@ -32,19 +30,19 @@ export function createTippy(target, opts = {}) {
}
}
visibleInstances.add(instance);
return optOnShow?.(instance);
return onShow?.(instance);
},
arrow: `<svg width="16" height="7"><path d="m0 7 8-7 8 7Z" class="tippy-svg-arrow-outer"/><path d="m0 8 8-7 8 7Z" class="tippy-svg-arrow-inner"/></svg>`,
role: 'menu', // HTML role attribute, only tooltips should use "tooltip"
theme: role || 'menu', // CSS theme, we support either "tooltip" or "menu"
...opts,
theme: other.role || 'menu', // CSS theme, we support either "tooltip" or "menu"
...other,
});

// for popups where content refers to a DOM element, we use the 'tippy-target' class
// to initially hide the content, now we can remove it as the content has been removed
// from the DOM by tippy
if (content instanceof Element) {
content.classList.remove('tippy-target');
if (other.content instanceof Element) {
other.content.classList.remove('tippy-target');
}

return instance;
Expand Down