Extract ModalManagerState from ModalManager (#2162)

js/src/admin/components/AppearancePage.js

@@ -82,7 +82,7 @@ export default class AppearancePage extends Page {
             {Button.component({
               className: 'Button',
               children: app.translator.trans('core.admin.appearance.edit_header_button'),
-              onclick: () => app.modal.show(new EditCustomHeaderModal()),
+              onclick: () => app.modal.show(EditCustomHeaderModal),
             })}
           </fieldset>
 
@@ -92,7 +92,7 @@ export default class AppearancePage extends Page {
             {Button.component({
               className: 'Button',
               children: app.translator.trans('core.admin.appearance.edit_footer_button'),
-              onclick: () => app.modal.show(new EditCustomFooterModal()),
+              onclick: () => app.modal.show(EditCustomFooterModal),
             })}
           </fieldset>
 
@@ -102,7 +102,7 @@ export default class AppearancePage extends Page {
             {Button.component({
               className: 'Button',
               children: app.translator.trans('core.admin.appearance.edit_css_button'),
-              onclick: () => app.modal.show(new EditCustomCssModal()),
+              onclick: () => app.modal.show(EditCustomCssModal),
             })}
           </fieldset>
         </div>

js/src/admin/components/ExtensionsPage.js

@@ -16,7 +16,7 @@ export default class ExtensionsPage extends Page {
               children: app.translator.trans('core.admin.extensions.add_button'),
               icon: 'fas fa-plus',
               className: 'Button Button--primary',
-              onclick: () => app.modal.show(new AddExtensionModal()),
+              onclick: () => app.modal.show(AddExtensionModal),
             })}
           </div>
         </div>
@@ -94,7 +94,7 @@ export default class ExtensionsPage extends Page {
               })
               .then(() => window.location.reload());
 
-            app.modal.show(new LoadingModal());
+            app.modal.show(LoadingModal);
           },
         })
       );
@@ -123,6 +123,6 @@ export default class ExtensionsPage extends Page {
         window.location.reload();
       });
 
-    app.modal.show(new LoadingModal());
+    app.modal.show(LoadingModal);
   }
 }

js/src/admin/components/LoadingModal.js

@@ -1,9 +1,10 @@
 import Modal from '../../common/components/Modal';
 
 export default class LoadingModal extends Modal {
-  isDismissible() {
-    return false;
-  }
+  /**
+   * @inheritdoc
+   */
+  static isDismissible = false;
 
   className() {
     return 'LoadingModal Modal--small';

js/src/admin/components/PermissionsPage.js

@@ -15,7 +15,7 @@ export default class PermissionsPage extends Page {
               .all('groups')
               .filter((group) => [Group.GUEST_ID, Group.MEMBER_ID].indexOf(group.id()) === -1)
               .map((group) => (
-                <button className="Button Group" onclick={() => app.modal.show(new EditGroupModal({ group }))}>
+                <button className="Button Group" onclick={() => app.modal.show(EditGroupModal, { group })}>
                   {GroupBadge.component({
                     group,
                     className: 'Group-icon',
@@ -24,7 +24,7 @@ export default class PermissionsPage extends Page {
                   <span className="Group-name">{group.namePlural()}</span>
                 </button>
               ))}
-            <button className="Button Group Group--add" onclick={() => app.modal.show(new EditGroupModal())}>
+            <button className="Button Group Group--add" onclick={() => app.modal.show(EditGroupModal)}>
               {icon('fas fa-plus', { className: 'Group-icon' })}
               <span className="Group-name">{app.translator.trans('core.admin.permissions.new_group_button')}</span>
             </button>

js/src/admin/components/StatusWidget.js

@@ -46,7 +46,7 @@ export default class StatusWidget extends DashboardWidget {
   }
 
   handleClearCache(e) {
-    app.modal.show(new LoadingModal());
+    app.modal.show(LoadingModal);
 
     app
       .request({

js/src/common/Application.js

@@ -22,6 +22,7 @@ import Group from './models/Group';
 import Notification from './models/Notification';
 import { flattenDeep } from 'lodash-es';
 import PageState from './states/PageState';
+import ModalManagerState from './states/ModalManagerState';
 import AlertManagerState from './states/AlertManagerState';
 
 /**
@@ -140,7 +141,16 @@ export default class Application {
   previous = new PageState(null);
 
   /*
+   * An object that manages modal state.
+   *
+   * @type {ModalManagerState}
+   */
+  modal = new ModalManagerState();
+
+  /**
    * An object that manages the state of active alerts.
+   *
+   * @type {AlertManagerState}
    */
   alerts = new AlertManagerState();
 
@@ -179,7 +189,7 @@ export default class Application {
   }
 
   mount(basePath = '') {
-    this.modal = m.mount(document.getElementById('modal'), <ModalManager />);
+    m.mount(document.getElementById('modal'), <ModalManager state={this.modal} />);
     m.mount(document.getElementById('alerts'), <AlertManager state={this.alerts} />);
 
     this.drawer = new Drawer();
@@ -402,7 +412,7 @@ export default class Application {
   showDebug(error, formattedError) {
     this.alerts.dismiss(this.requestErrorAlert);
 
-    this.modal.show(new RequestErrorModal({ error, formattedError }));
+    this.modal.show(RequestErrorModal, { error, formattedError });
   }
 
   /**

js/src/common/components/Modal.js

@@ -9,6 +9,11 @@ import Button from './Button';
  * @abstract
  */
 export default class Modal extends Component {
+  /**
+   * Determine whether or not the modal should be dismissible via an 'x' button.
+   */
+  static isDismissible = true;
+
   init() {
     /**
      * Attributes for an alert component to show below the header.
@@ -18,6 +23,16 @@ export default class Modal extends Component {
     this.alertAttrs = null;
   }
 
+  config(isInitialized, context) {
+    if (isInitialized) return;
+
+    this.props.onshow(() => this.onready());
+
+    context.onunload = () => {
+      this.props.onhide();
+    };
+  }
+
   view() {
     if (this.alertAttrs) {
       this.alertAttrs.dismissible = false;
@@ -26,7 +41,7 @@ export default class Modal extends Component {
     return (
       <div className={'Modal modal-dialog ' + this.className()}>
         <div className="Modal-content">
-          {this.isDismissible() ? (
+          {this.constructor.isDismissible ? (
             <div className="Modal-close App-backControl">
               {Button.component({
                 icon: 'fas fa-times',
@@ -52,15 +67,6 @@ export default class Modal extends Component {
     );
   }
 
-  /**
-   * Determine whether or not the modal should be dismissible via an 'x' button.
-   *
-   * @return {Boolean}
-   */
-  isDismissible() {
-    return true;
-  }
-
   /**
    * Get the class name to apply to the modal.
    *
@@ -105,7 +111,7 @@ export default class Modal extends Component {
    * Hide the modal.
    */
   hide() {
-    app.modal.close();
+    this.props.onhide();
   }
 
   /**

js/src/common/components/ModalManager.js

@@ -1,5 +1,4 @@
 import Component from '../Component';
-import Modal from './Modal';
 
 /**
  * The `ModalManager` component manages a modal dialog. Only one modal dialog
@@ -8,12 +7,17 @@ import Modal from './Modal';
  */
 export default class ModalManager extends Component {
   init() {
-    this.showing = false;
-    this.component = null;
+    this.state = this.props.state;
   }
 
   view() {
-    return <div className="ModalManager modal fade">{this.component && this.component.render()}</div>;
+    const modal = this.state.modal;
+
+    return (
+      <div className="ModalManager modal fade">
+        {modal ? modal.componentClass.component({ ...modal.attrs, onshow: this.animateShow.bind(this), onhide: this.animateHide.bind(this) }) : ''}
+      </div>
+    );
   }
 
   config(isInitialized, context) {
@@ -24,29 +28,17 @@ export default class ModalManager extends Component {
     // to be retained across route changes.
     context.retain = true;
 
-    this.$().on('hidden.bs.modal', this.clear.bind(this)).on('shown.bs.modal', this.onready.bind(this));
+    // Ensure the modal state is notified about a closed modal, even when the
+    // DOM-based Bootstrap JavaScript code triggered the closing of the modal,
+    // e.g. via ESC key or a click on the modal backdrop.
+    this.$().on('hidden.bs.modal', this.state.close.bind(this.state));
   }
 
-  /**
-   * Show a modal dialog.
-   *
-   * @param {Modal} component
-   * @public
-   */
-  show(component) {
-    if (!(component instanceof Modal)) {
-      throw new Error('The ModalManager component can only show Modal components');
-    }
+  animateShow(readyCallback) {
+    const dismissible = !!this.state.modal.componentClass.isDismissible;
 
-    clearTimeout(this.hideTimeout);
-
-    this.showing = true;
-    this.component = component;
-
-    m.redraw(true);
-
-    const dismissible = !!this.component.isDismissible();
     this.$()
+      .one('shown.bs.modal', readyCallback)
       .modal({
         backdrop: dismissible || 'static',
         keyboard: dismissible,
@@ -54,50 +46,7 @@ export default class ModalManager extends Component {
       .modal('show');
   }
 
-  /**
-   * Close the modal dialog.
-   *
-   * @public
-   */
-  close() {
-    if (!this.showing) return;
-
-    // Don't hide the modal immediately, because if the consumer happens to call
-    // the `show` method straight after to show another modal dialog, it will
-    // cause Bootstrap's modal JS to misbehave. Instead we will wait for a tiny
-    // bit to give the `show` method the opportunity to prevent this from going
-    // ahead.
-    this.hideTimeout = setTimeout(() => {
+  animateHide() {
     this.$().modal('hide');
-      this.showing = false;
-    });
-  }
-
-  /**
-   * Clear content from the modal area.
-   *
-   * @protected
-   */
-  clear() {
-    if (this.component) {
-      this.component.onhide();
-    }
-
-    this.component = null;
-
-    app.current.retain = false;
-
-    m.lazyRedraw();
-  }
-
-  /**
-   * When the modal dialog is ready to be used, tell it!
-   *
-   * @protected
-   */
-  onready() {
-    if (this.component && this.component.onready) {
-      this.component.onready(this.$());
-    }
   }
 }

js/src/common/states/ModalManagerState.js

@@ -0,0 +1,56 @@
+import Modal from '../components/Modal';
+
+export default class ModalManagerState {
+  constructor() {
+    this.modal = null;
+  }
+
+  /**
+   * Show a modal dialog.
+   *
+   * @public
+   */
+  show(componentClass, attrs) {
+    // Breaking Change Compliance Warning, Remove in Beta 15.
+    if (!(componentClass.prototype instanceof Modal)) {
+      // This is duplicated so that if the error is caught, an error message still shows up in the debug console.
+      console.error('The ModalManager can only show Modals');
+      throw new Error('The ModalManager can only show Modals');
+    }
+    if (componentClass.init) {
+      // This is duplicated so that if the error is caught, an error message still shows up in the debug console.
+      console.error(
+        'The componentClass parameter must be a modal class, not a modal instance. Whichever extension triggered this modal should be updated to comply with beta 14.'
+      );
+      throw new Error(
+        'The componentClass parameter must be a modal class, not a modal instance. Whichever extension triggered this modal should be updated to comply with beta 14.'
+      );
+    }
+    // End Change Compliance Warning, Remove in Beta 15
+
+    clearTimeout(this.closeTimeout);
+
+    this.modal = { componentClass, attrs };
+
+    m.redraw(true);
+  }
+
+  /**
+   * Close the modal dialog.
+   *
+   * @public
+   */
+  close() {
+    if (!this.modal) return;
+
+    // Don't hide the modal immediately, because if the consumer happens to call
+    // the `show` method straight after to show another modal dialog, it will
+    // cause Bootstrap's modal JS to misbehave. Instead we will wait for a tiny
+    // bit to give the `show` method the opportunity to prevent this from going
+    // ahead.
+    this.closeTimeout = setTimeout(() => {
+      this.modal = null;
+      m.lazyRedraw();
+    });
+  }
+}

js/src/forum/ForumApplication.js

@@ -180,8 +180,7 @@ export default class ForumApplication extends Application {
     if (payload.loggedIn) {
       window.location.reload();
     } else {
-      const modal = new SignUpModal(payload);
-      this.modal.show(modal);
+      this.modal.show(SignUpModal, payload);
     }
   }
 }

js/src/forum/components/HeaderSecondary.js

@@ -77,7 +77,7 @@ export default class HeaderSecondary extends Component {
           Button.component({
             children: app.translator.trans('core.forum.header.sign_up_link'),
             className: 'Button Button--link',
-            onclick: () => app.modal.show(new SignUpModal()),
+            onclick: () => app.modal.show(SignUpModal),
           }),
           10
         );
@@ -88,7 +88,7 @@ export default class HeaderSecondary extends Component {
         Button.component({
           children: app.translator.trans('core.forum.header.log_in_link'),
           className: 'Button Button--link',
-          onclick: () => app.modal.show(new LogInModal()),
+          onclick: () => app.modal.show(LogInModal),
         }),
         0
       );

js/src/forum/components/IndexPage.js

@@ -282,7 +282,7 @@ export default class IndexPage extends Page {
     } else {
       deferred.reject();
 
-      app.modal.show(new LogInModal());
+      app.modal.show(LogInModal);
     }
 
     return deferred.promise;

js/src/forum/components/LogInModal.js

@@ -142,7 +142,7 @@ export default class LogInModal extends Modal {
     const email = this.identification();
     const props = email.indexOf('@') !== -1 ? { email } : undefined;
 
-    app.modal.show(new ForgotPasswordModal(props));
+    app.modal.show(ForgotPasswordModal, props);
   }
 
   /**
@@ -156,7 +156,7 @@ export default class LogInModal extends Modal {
     const identification = this.identification();
     props[identification.indexOf('@') !== -1 ? 'email' : 'username'] = identification;
 
-    app.modal.show(new SignUpModal(props));
+    app.modal.show(SignUpModal, props);
   }
 
   onready() {

js/src/forum/components/SettingsPage.js

@@ -79,7 +79,7 @@ export default class SettingsPage extends UserPage {
       Button.component({
         children: app.translator.trans('core.forum.settings.change_password_button'),
         className: 'Button',
-        onclick: () => app.modal.show(new ChangePasswordModal()),
+        onclick: () => app.modal.show(ChangePasswordModal),
       })
     );
 
@@ -88,7 +88,7 @@ export default class SettingsPage extends UserPage {
       Button.component({
         children: app.translator.trans('core.forum.settings.change_email_button'),
         className: 'Button',
-        onclick: () => app.modal.show(new ChangeEmailModal()),
+        onclick: () => app.modal.show(ChangeEmailModal),
       })
     );
 

js/src/forum/components/SignUpModal.js

@@ -145,7 +145,7 @@ export default class SignUpModal extends Modal {
       password: this.password(),
     };
 
-    app.modal.show(new LogInModal(props));
+    app.modal.show(LogInModal, props);
   }
 
   onready() {

js/src/forum/utils/DiscussionControls.js

@@ -188,7 +188,7 @@ export default {
     } else {
       deferred.reject();
 
-      app.modal.show(new LogInModal());
+      app.modal.show(LogInModal);
     }
 
     return deferred.promise;
@@ -239,11 +239,9 @@ export default {
    * @return {Promise}
    */
   renameAction() {
-    return app.modal.show(
-      new RenameDiscussionModal({
+    return app.modal.show(RenameDiscussionModal, {
       currentTitle: this.title(),
       discussion: this,
-      })
-    );
+    });
   },
 };

js/src/forum/utils/UserControls.js

@@ -145,6 +145,6 @@ export default {
    * @param {User} user
    */
   editAction(user) {
-    app.modal.show(new EditUserModal({ user }));
+    app.modal.show(EditUserModal, { user });
   },
 };