Dismissing the required-comment modal no longer looks like a close (#20)
Lint / JS (eslint) (push) Successful in 15s
Lint / PHP requirements (version + extensions) (push) Successful in 41s
Security / PHP Security (semgrep) (push) Successful in 1m8s
Lint / PHP (phpcs PSR-12) (push) Successful in 18s
Lint / Notify on failure (push) Skipped
Lint / Deploy (push) Successful in 2s
Lint / JS (eslint) (push) Successful in 15s
Lint / PHP requirements (version + extensions) (push) Successful in 41s
Security / PHP Security (semgrep) (push) Successful in 1m8s
Lint / PHP (phpcs PSR-12) (push) Successful in 18s
Lint / Notify on failure (push) Skipped
Lint / Deploy (push) Successful in 2s
A modal can be dismissed four ways: the ✕ button, Cancel, a backdrop click, or
Escape. base.js handles the last two globally (a document click handler and
registerKey('escape', closeAllModals)), so the status-change modal — which wired
only the two buttons — never learned it had been dismissed. The status dropdown
kept displaying the new status even though update_ticket.php was never called,
so the ticket looked closed with no comment until a reload showed it still open.
The same gap left every dynamically-inserted modal in the DOM when dismissed
that way, so the next open inserted a duplicate id that shadowed the live one.
- base.js closeModal now dispatches a bubbling lt:modalclose event (synced to
web_template as bbec859), and _statusCommentModal treats it as "no comment".
- ticket.js reverts the dropdown on any dismissal, guarded against the re-entry
its own lt.modal.close() would otherwise cause.
- dashboard.js gains openModalWithDismiss() so all seven dynamic modals plus the
generic prompt modal tear down however they are dismissed.
Verified in headless chromium against all four dismissal routes plus a
confirm-with-comment control: 22/22. Against the pre-fix files the same test
fails 6 assertions — backdrop and Escape leave the dropdown on "Closed *" with
an orphaned overlay — so it reproduces the reported behaviour exactly.
This commit is contained in:
@@ -241,6 +241,11 @@
|
||||
trigger.focus();
|
||||
}
|
||||
}
|
||||
// Announce the close so whoever opened the modal can undo optimistic UI or
|
||||
// clean up a dynamically-inserted overlay. A modal can be dismissed four
|
||||
// ways — the ✕ button, a Cancel button, a backdrop click, and Escape — and
|
||||
// the last two are handled globally here, so button-only listeners miss them.
|
||||
el.dispatchEvent(new CustomEvent('lt:modalclose', { bubbles: true }));
|
||||
}
|
||||
|
||||
function closeAllModals() {
|
||||
@@ -2774,6 +2779,9 @@
|
||||
setTimeout(() => { if (modalEl && modalEl.parentNode) modalEl.remove(); }, 300);
|
||||
resolve(value);
|
||||
};
|
||||
// Any dismissal counts as "no comment given", including a backdrop click or
|
||||
// Escape, which close the overlay through the global handlers above.
|
||||
modalEl.addEventListener('lt:modalclose', () => finish(null));
|
||||
modalEl.querySelector('[data-modal-close]').addEventListener('click', () => finish(null));
|
||||
document.getElementById(modalId + '_cancel').addEventListener('click', () => finish(null));
|
||||
document.getElementById(modalId + '_confirm').addEventListener('click', () => {
|
||||
|
||||
+23
-7
@@ -550,7 +550,7 @@ function bulkClose() {
|
||||
`;
|
||||
|
||||
document.body.insertAdjacentHTML('beforeend', modalHtml);
|
||||
lt.modal.open('bulkCloseModal');
|
||||
openModalWithDismiss('bulkCloseModal', closeBulkCloseModal);
|
||||
}
|
||||
|
||||
function closeBulkCloseModal() {
|
||||
@@ -633,7 +633,7 @@ function showBulkAssignModal() {
|
||||
`;
|
||||
|
||||
document.body.insertAdjacentHTML('beforeend', modalHtml);
|
||||
lt.modal.open('bulkAssignModal');
|
||||
openModalWithDismiss('bulkAssignModal', closeBulkAssignModal);
|
||||
setTimeout(() => { const inp = document.getElementById('bulkAssignUserInput'); if (inp) inp.focus(); }, 120);
|
||||
|
||||
lt.api.get('/api/get_users.php')
|
||||
@@ -731,7 +731,7 @@ function showBulkPriorityModal() {
|
||||
`;
|
||||
|
||||
document.body.insertAdjacentHTML('beforeend', modalHtml);
|
||||
lt.modal.open('bulkPriorityModal');
|
||||
openModalWithDismiss('bulkPriorityModal', closeBulkPriorityModal);
|
||||
}
|
||||
|
||||
function closeBulkPriorityModal() {
|
||||
@@ -845,7 +845,7 @@ function showBulkStatusModal() {
|
||||
`;
|
||||
|
||||
document.body.insertAdjacentHTML('beforeend', modalHtml);
|
||||
lt.modal.open('bulkStatusModal');
|
||||
openModalWithDismiss('bulkStatusModal', closeBulkStatusModal);
|
||||
}
|
||||
|
||||
function closeBulkStatusModal() {
|
||||
@@ -942,7 +942,7 @@ function showBulkDeleteModal() {
|
||||
`;
|
||||
|
||||
document.body.insertAdjacentHTML('beforeend', modalHtml);
|
||||
lt.modal.open('bulkDeleteModal');
|
||||
openModalWithDismiss('bulkDeleteModal', closeBulkDeleteModal);
|
||||
}
|
||||
|
||||
function closeBulkDeleteModal() {
|
||||
@@ -1032,6 +1032,22 @@ function showInputModal(title, label, placeholder = '', onSubmit, onCancel = nul
|
||||
input.addEventListener('keypress', (e) => { if (e.key === 'Enter') handleSubmit(); });
|
||||
document.getElementById(`${modalId}_cancel`).addEventListener('click', () => cleanup(onCancel));
|
||||
modal.querySelector('[data-modal-close]').addEventListener('click', () => cleanup(onCancel));
|
||||
// Backdrop click / Escape close the overlay via base.js's global handlers.
|
||||
modal.addEventListener('lt:modalclose', () => cleanup(onCancel));
|
||||
}
|
||||
|
||||
/**
|
||||
* Open a dynamically-inserted modal and make sure it tears itself down however it
|
||||
* is dismissed. base.js handles backdrop clicks and Escape globally, so wiring
|
||||
* only the ✕/Cancel buttons leaves the overlay in the DOM — and the next open
|
||||
* inserts a second element with the same id, which then shadows the live one.
|
||||
*/
|
||||
function openModalWithDismiss(modalId, onDismiss) {
|
||||
lt.modal.open(modalId);
|
||||
const el = document.getElementById(modalId);
|
||||
// lt.modal.close() early-returns once .is-open is gone, so the close call
|
||||
// inside onDismiss cannot re-enter this listener.
|
||||
if (el) el.addEventListener('lt:modalclose', onDismiss);
|
||||
}
|
||||
|
||||
// ========================================
|
||||
@@ -1069,7 +1085,7 @@ function quickStatusChange(ticketId, currentStatus) {
|
||||
`;
|
||||
|
||||
document.body.insertAdjacentHTML('beforeend', modalHtml);
|
||||
lt.modal.open('quickStatusModal');
|
||||
openModalWithDismiss('quickStatusModal', closeQuickStatusModal);
|
||||
}
|
||||
|
||||
function closeQuickStatusModal() {
|
||||
@@ -1136,7 +1152,7 @@ function quickAssign(ticketId) {
|
||||
`;
|
||||
|
||||
document.body.insertAdjacentHTML('beforeend', modalHtml);
|
||||
lt.modal.open('quickAssignModal');
|
||||
openModalWithDismiss('quickAssignModal', closeQuickAssignModal);
|
||||
|
||||
lt.api.get('/api/get_users.php')
|
||||
.then(data => {
|
||||
|
||||
+13
-1
@@ -529,7 +529,19 @@ function updateTicketStatus() {
|
||||
`);
|
||||
const modal = document.getElementById(modalId);
|
||||
lt.modal.open(modalId);
|
||||
const cleanup = (ok) => { lt.modal.close(modalId); setTimeout(() => modal.remove(), 300); if (!ok) statusSelect.selectedIndex = 0; };
|
||||
let settled = false;
|
||||
const cleanup = (ok) => {
|
||||
if (settled) return; // lt.modal.close() below re-enters via lt:modalclose
|
||||
settled = true;
|
||||
lt.modal.close(modalId);
|
||||
setTimeout(() => modal.remove(), 300);
|
||||
if (!ok) statusSelect.selectedIndex = 0;
|
||||
};
|
||||
// Backdrop click and Escape close the overlay through base.js's global
|
||||
// handlers. Without this the dropdown kept displaying the new status
|
||||
// while the server was never called, so the ticket looked closed until
|
||||
// a reload revealed it was still open.
|
||||
modal.addEventListener('lt:modalclose', () => cleanup(false));
|
||||
modal.querySelector('[data-modal-close]').addEventListener('click', () => cleanup(false));
|
||||
document.getElementById(`${modalId}_cancel`).addEventListener('click', () => cleanup(false));
|
||||
document.getElementById(`${modalId}_confirm`).addEventListener('click', () => {
|
||||
|
||||
Reference in New Issue
Block a user