From 4f7db41030914238ce288cb3e18838a7f6e5fd18 Mon Sep 17 00:00:00 2001
From: Sage Abdullah <sage.abdullah@torchbox.com>
Date: Wed, 24 Jul 2024 17:27:09 +0100
Subject: [PATCH] Ensure submit buttons inside dialogs also trigger the
overwrite confirmation dialog
---
.../src/controllers/SessionController.test.js | 92 ++++++++++++++++++-
client/src/controllers/SessionController.ts | 39 +++++---
2 files changed, 115 insertions(+), 16 deletions(-)
diff --git a/client/src/controllers/SessionController.test.js b/client/src/controllers/SessionController.test.js
index b0eaca01f4..9b82737487 100644
--- a/client/src/controllers/SessionController.test.js
+++ b/client/src/controllers/SessionController.test.js
@@ -238,9 +238,10 @@ describe('SessionController', () => {
workflowActionButton.addEventListener('click', handleWorkflowAction, {
capture: true,
});
- document.addEventListener('w-dialog:shown', handleDialogShow);
- document.addEventListener('w-dialog:hidden', handleDialogHidden);
- document.addEventListener('w-dialog:confirmed', handleDialogConfirmed);
+ const dialog = document.getElementById('w-overwrite-changes-dialog');
+ dialog.addEventListener('w-dialog:shown', handleDialogShow);
+ dialog.addEventListener('w-dialog:hidden', handleDialogHidden);
+ dialog.addEventListener('w-dialog:confirmed', handleDialogConfirmed);
});
afterEach(() => {
@@ -410,6 +411,91 @@ describe('SessionController', () => {
expect(dialog.getAttribute('aria-hidden')).toBeNull();
expect(confirmButton.textContent).toEqual('Approve');
});
+
+ it("should hide the submit button's dialog when it shows the confirmation dialog and show it again afterwards", async () => {
+ const confirmButton = document.getElementById('confirm');
+ // Mark the confirm button as DialogController's confirm target
+ confirmButton.setAttribute('data-w-dialog-target', 'confirm');
+
+ const otherDialog = document.createElement('div');
+ otherDialog.id = 'w-schedule-publishing-dialog';
+ otherDialog.setAttribute('aria-hidden', 'true');
+ otherDialog.setAttribute('data-controller', 'w-dialog');
+ otherDialog.setAttribute(
+ 'data-action',
+ 'w-dialog:hide->w-dialog#hide w-dialog:show->w-dialog#show',
+ );
+ otherDialog.innerHTML = /* html */ `
+ <div role="document">
+ <div id="schedule-publishing-dialog-body" data-w-dialog-target="body">
+ Set the publishing schedule
+
+ <input type="datetime-local" name="go_live_at" />
+ <input type="datetime-local" name="expire_at" />
+
+ <button type="submit">Save schedule</button>
+ </div>
+ </div>
+ `;
+
+ const otherDialogTrigger = document.createElement('button');
+ otherDialogTrigger.type = 'button';
+ otherDialogTrigger.setAttribute(
+ 'data-a11y-dialog-show',
+ 'w-schedule-publishing-dialog',
+ );
+ otherDialogTrigger.innerHTML = 'Set schedule';
+
+ form.appendChild(otherDialog);
+ form.appendChild(otherDialogTrigger);
+
+ // Reconnect the DialogController so the submit button in the
+ // schedule publishing dialog works
+ const dialog = document.querySelector('#w-overwrite-changes-dialog');
+ dialog.removeAttribute('data-controller');
+ await Promise.resolve();
+ dialog.setAttribute('data-controller', 'w-dialog');
+ await Promise.resolve();
+
+ expect(handleDialogShow).not.toHaveBeenCalled();
+
+ // Show the schedule publishing dialog
+ otherDialogTrigger.click();
+ expect(otherDialog.getAttribute('aria-hidden')).toBeNull();
+
+ // Should not trigger the confirmation dialog yet
+ expect(handleDialogShow).not.toHaveBeenCalled();
+
+ const scheduleSubmitButton = otherDialog.querySelector(
+ 'button[type="submit"]',
+ );
+ scheduleSubmitButton.click();
+ await Promise.resolve();
+
+ // Should trigger the confirmation dialog
+ expect(handleSubmit).not.toHaveBeenCalled();
+ expect(handleWorkflowAction).not.toHaveBeenCalled();
+ expect(handleDialogShow).toHaveBeenCalled();
+ expect(dialog.getAttribute('aria-hidden')).toBeNull();
+ expect(confirmButton.textContent).toEqual('Save schedule');
+
+ // Should hide the schedule publishing dialog
+ expect(otherDialog.getAttribute('aria-hidden')).toEqual('true');
+
+ // Confirm the dialog
+ confirmButton.click();
+ await Promise.resolve();
+
+ // Should hide the confirmation dialog and continue the action
+ expect(handleDialogHidden).toHaveBeenCalled();
+ expect(handleDialogConfirmed).toHaveBeenCalled();
+ expect(handleSubmit).toHaveBeenCalledTimes(1);
+ expect(handleWorkflowAction).not.toHaveBeenCalled();
+ expect(dialog.getAttribute('aria-hidden')).toEqual('true');
+
+ // The schedule publishing dialog should still be hidden
+ expect(otherDialog.getAttribute('aria-hidden')).toEqual('true');
+ });
});
describe('storing unsaved changes state to a checkbox input and update reload buttons accordingly', () => {
diff --git a/client/src/controllers/SessionController.ts b/client/src/controllers/SessionController.ts
index 918cc9249c..cbccee9157 100644
--- a/client/src/controllers/SessionController.ts
+++ b/client/src/controllers/SessionController.ts
@@ -87,14 +87,6 @@ export class SessionController extends Controller<HTMLElement> {
}
connect(): void {
- this.interceptTargets.forEach((button) => {
- // Match the event listener configuration of workflow-action that uses
- // capture so we can intercept the workflow-action's listener.
- button.addEventListener('click', this.showConfirmationDialog, {
- capture: true,
- });
- });
-
// Do a ping so the sessions list can be loaded immediately.
this.ping();
}
@@ -175,6 +167,12 @@ export class SessionController extends Controller<HTMLElement> {
// workflow action modal) after the user confirms the dialog.
if (!this.interceptValue || !this.hasWDialogOutlet) return;
+ // If the action button is inside a dialog, we need to hide the dialog first
+ // so it doesn't interfere with the confirmation dialog
+ this.lastActionButton
+ ?.closest('[data-controller="w-dialog"]')
+ ?.dispatchEvent(new Event('w-dialog:hide'));
+
// Prevent form submission
event.preventDefault();
// Prevent triggering other event listeners e.g. workflow actions modal
@@ -197,12 +195,32 @@ export class SessionController extends Controller<HTMLElement> {
}
wDialogOutletConnected(): void {
+ // Attach the event listener to the buttons that will be intercepted.
+ // Do it here instead of in connect() so hopefully this includes any buttons
+ // that are inside a dialog that is connected after this controller
+ // (e.g. the schedule publishing dialog).
+ this.interceptTargets.forEach((button) => {
+ // Match the event listener configuration of workflow-action that uses
+ // capture so we can intercept the workflow-action's listener.
+ button.addEventListener('click', this.showConfirmationDialog, {
+ capture: true,
+ });
+ });
+
this.wDialogOutlet.element.addEventListener(
'w-dialog:confirmed',
this.confirmAction,
);
}
+ wDialogOutletDisconnected(): void {
+ this.interceptTargets?.forEach((button) => {
+ button.removeEventListener('click', this.showConfirmationDialog, {
+ capture: true,
+ });
+ });
+ }
+
/**
* Sets the unsaved changes input state based on the event type dispatched by
* the w-unsaved controller. If the event type is w-unsaved:add, the input is
@@ -293,10 +311,5 @@ export class SessionController extends Controller<HTMLElement> {
if (this.interval) {
window.clearInterval(this.interval);
}
- this.interceptTargets?.forEach((button) => {
- button.removeEventListener('click', this.showConfirmationDialog, {
- capture: true,
- });
- });
}
}