Skip to content

Commit 6596b92

Browse files
authored
Fix modal + form abuse (#34921)
See the comment. And due to the abuse, there is a regression: when the modal is hidden, the form will be reset and it can't submit. This PR fixes all problems: keep the modal with form open, and add "loading" indicator.
1 parent f3364ec commit 6596b92

File tree

3 files changed

+41
-16
lines changed

3 files changed

+41
-16
lines changed

web_src/js/features/common-button.ts

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,16 @@ export function initGlobalDeleteButton(): void {
4343

4444
fomanticQuery(modal).modal({
4545
closable: false,
46-
onApprove: async () => {
46+
onApprove: () => {
4747
// if `data-type="form"` exists, then submit the form by the selector provided by `data-form="..."`
4848
if (btn.getAttribute('data-type') === 'form') {
4949
const formSelector = btn.getAttribute('data-form');
5050
const form = document.querySelector<HTMLFormElement>(formSelector);
5151
if (!form) throw new Error(`no form named ${formSelector} found`);
52+
modal.classList.add('is-loading'); // the form is not in the modal, so also add loading indicator to the modal
53+
form.classList.add('is-loading');
5254
form.submit();
55+
return false; // prevent modal from closing automatically
5356
}
5457

5558
// prepare an AJAX form by data attributes
@@ -62,12 +65,15 @@ export function initGlobalDeleteButton(): void {
6265
postData.append('id', value);
6366
}
6467
}
65-
66-
const response = await POST(btn.getAttribute('data-url'), {data: postData});
67-
if (response.ok) {
68-
const data = await response.json();
69-
window.location.href = data.redirect;
70-
}
68+
(async () => {
69+
const response = await POST(btn.getAttribute('data-url'), {data: postData});
70+
if (response.ok) {
71+
const data = await response.json();
72+
window.location.href = data.redirect;
73+
}
74+
})();
75+
modal.classList.add('is-loading'); // the request is in progress, so also add loading indicator to the modal
76+
return false; // prevent modal from closing automatically
7177
},
7278
}).modal('show');
7379
});
@@ -158,13 +164,7 @@ function onShowModalClick(el: HTMLElement, e: MouseEvent) {
158164
}
159165
}
160166

161-
fomanticQuery(elModal).modal('setting', {
162-
onApprove: () => {
163-
// "form-fetch-action" can handle network errors gracefully,
164-
// so keep the modal dialog to make users can re-submit the form if anything wrong happens.
165-
if (elModal.querySelector('.form-fetch-action')) return false;
166-
},
167-
}).modal('show');
167+
fomanticQuery(elModal).modal('show');
168168
}
169169

170170
export function initGlobalButtons(): void {

web_src/js/features/comp/LabelEdit.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ export function initCompLabelEdit(pageSelector: string) {
7272
return false;
7373
}
7474
submitFormFetchAction(form);
75+
return false;
7576
},
7677
}).modal('show');
7778
};

web_src/js/modules/fomantic/modal.ts

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@ const fomanticModalFn = $.fn.modal;
99
export function initAriaModalPatch() {
1010
if ($.fn.modal === ariaModalFn) throw new Error('initAriaModalPatch could only be called once');
1111
$.fn.modal = ariaModalFn;
12-
$.fn.fomanticExt.onModalBeforeHidden = onModalBeforeHidden;
1312
(ariaModalFn as FomanticInitFunction).settings = fomanticModalFn.settings;
13+
$.fn.fomanticExt.onModalBeforeHidden = onModalBeforeHidden;
14+
$.fn.modal.settings.onApprove = onModalApproveDefault;
1415
}
1516

1617
// the patched `$.fn.modal` modal function
@@ -34,6 +35,29 @@ function ariaModalFn(this: any, ...args: Parameters<FomanticInitFunction>) {
3435
function onModalBeforeHidden(this: any) {
3536
const $modal = $(this);
3637
const elModal = $modal[0];
37-
queryElems(elModal, 'form', (form: HTMLFormElement) => form.reset());
3838
hideToastsFrom(elModal.closest('.ui.dimmer') ?? document.body);
39+
40+
// reset the form after the modal is hidden, after other modal events and handlers (e.g. "onApprove", form submit)
41+
setTimeout(() => {
42+
queryElems(elModal, 'form', (form: HTMLFormElement) => form.reset());
43+
}, 0);
44+
}
45+
46+
function onModalApproveDefault(this: any) {
47+
const $modal = $(this);
48+
const selectors = $modal.modal('setting', 'selector');
49+
const elModal = $modal[0];
50+
const elApprove = elModal.querySelector(selectors.approve);
51+
const elForm = elApprove?.closest('form');
52+
if (!elForm) return true; // no form, just allow closing the modal
53+
54+
// "form-fetch-action" can handle network errors gracefully,
55+
// so keep the modal dialog to make users can re-submit the form if anything wrong happens.
56+
if (elForm.matches('.form-fetch-action')) return false;
57+
58+
// There is an abuse for the "modal" + "form" combination, the "Approve" button is a traditional form submit button in the form.
59+
// Then "approve" and "submit" occur at the same time, the modal will be closed immediately before the form is submitted.
60+
// So here we prevent the modal from closing automatically by returning false, add the "is-loading" class to the form element.
61+
elForm.classList.add('is-loading');
62+
return false;
3963
}

0 commit comments

Comments
 (0)