From 1c1a3b363ecefbce90c2fa99e5184f74f548b5a4 Mon Sep 17 00:00:00 2001
From: Alexander Skvortsov <38059171+askvortsov1@users.noreply.github.com>
Date: Fri, 19 Jun 2020 17:41:26 -0400
Subject: [PATCH] Clean up app.current, app.previous in JS (#2156)

- Encapsulate app.current, app.previous in PageState objects
- Reorganize Page classes to use one central base class in common

Co-authored-by: Franz Liedke <franz@develophp.org>
---
 framework/core/js/src/admin/compat.js         |  2 --
 .../js/src/admin/components/AppearancePage.js |  2 +-
 .../js/src/admin/components/BasicsPage.js     |  2 +-
 .../js/src/admin/components/DashboardPage.js  |  2 +-
 .../js/src/admin/components/ExtensionsPage.js |  5 +--
 .../core/js/src/admin/components/MailPage.js  |  2 +-
 .../core/js/src/admin/components/Page.js      | 32 ------------------
 .../src/admin/components/PermissionsPage.js   |  2 +-
 framework/core/js/src/common/Application.js   | 23 +++++++++++++
 framework/core/js/src/common/compat.js        |  2 ++
 .../js/src/common/components/ModalManager.js  |  2 --
 .../src/{forum => common}/components/Page.js  |  5 +--
 .../core/js/src/common/states/PageState.js    | 33 +++++++++++++++++++
 .../core/js/src/common/utils/subclassOf.js    |  6 ++++
 .../core/js/src/forum/ForumApplication.js     |  2 +-
 framework/core/js/src/forum/compat.js         |  2 --
 .../js/src/forum/components/DiscussionPage.js |  7 ++--
 .../core/js/src/forum/components/IndexPage.js |  8 ++---
 .../src/forum/components/NotificationsPage.js |  2 +-
 .../core/js/src/forum/components/Post.js      |  3 +-
 .../forum/components/RenameDiscussionModal.js |  2 +-
 .../js/src/forum/components/ReplyComposer.js  |  3 +-
 .../core/js/src/forum/components/UserPage.js  |  4 ++-
 .../js/src/forum/states/GlobalSearchState.js  |  2 +-
 .../js/src/forum/utils/DiscussionControls.js  |  2 +-
 .../core/js/src/forum/utils/UserControls.js   |  2 +-
 26 files changed, 95 insertions(+), 64 deletions(-)
 delete mode 100644 framework/core/js/src/admin/components/Page.js
 rename framework/core/js/src/{forum => common}/components/Page.js (80%)
 create mode 100644 framework/core/js/src/common/states/PageState.js
 create mode 100644 framework/core/js/src/common/utils/subclassOf.js

diff --git a/framework/core/js/src/admin/compat.js b/framework/core/js/src/admin/compat.js
index ff3abdea7..84bc61671 100644
--- a/framework/core/js/src/admin/compat.js
+++ b/framework/core/js/src/admin/compat.js
@@ -6,7 +6,6 @@ import EditCustomFooterModal from './components/EditCustomFooterModal';
 import SessionDropdown from './components/SessionDropdown';
 import HeaderPrimary from './components/HeaderPrimary';
 import AppearancePage from './components/AppearancePage';
-import Page from './components/Page';
 import StatusWidget from './components/StatusWidget';
 import HeaderSecondary from './components/HeaderSecondary';
 import SettingsModal from './components/SettingsModal';
@@ -36,7 +35,6 @@ export default Object.assign(compat, {
   'components/SessionDropdown': SessionDropdown,
   'components/HeaderPrimary': HeaderPrimary,
   'components/AppearancePage': AppearancePage,
-  'components/Page': Page,
   'components/StatusWidget': StatusWidget,
   'components/HeaderSecondary': HeaderSecondary,
   'components/SettingsModal': SettingsModal,
diff --git a/framework/core/js/src/admin/components/AppearancePage.js b/framework/core/js/src/admin/components/AppearancePage.js
index 98a1cc373..9b130ef23 100644
--- a/framework/core/js/src/admin/components/AppearancePage.js
+++ b/framework/core/js/src/admin/components/AppearancePage.js
@@ -1,4 +1,4 @@
-import Page from './Page';
+import Page from '../../common/components/Page';
 import Button from '../../common/components/Button';
 import Switch from '../../common/components/Switch';
 import EditCustomCssModal from './EditCustomCssModal';
diff --git a/framework/core/js/src/admin/components/BasicsPage.js b/framework/core/js/src/admin/components/BasicsPage.js
index fb692c4fc..497a0fddb 100644
--- a/framework/core/js/src/admin/components/BasicsPage.js
+++ b/framework/core/js/src/admin/components/BasicsPage.js
@@ -1,4 +1,4 @@
-import Page from './Page';
+import Page from '../../common/components/Page';
 import FieldSet from '../../common/components/FieldSet';
 import Select from '../../common/components/Select';
 import Button from '../../common/components/Button';
diff --git a/framework/core/js/src/admin/components/DashboardPage.js b/framework/core/js/src/admin/components/DashboardPage.js
index 2e4c0593e..fdcade871 100644
--- a/framework/core/js/src/admin/components/DashboardPage.js
+++ b/framework/core/js/src/admin/components/DashboardPage.js
@@ -1,4 +1,4 @@
-import Page from './Page';
+import Page from '../../common/components/Page';
 import StatusWidget from './StatusWidget';
 
 export default class DashboardPage extends Page {
diff --git a/framework/core/js/src/admin/components/ExtensionsPage.js b/framework/core/js/src/admin/components/ExtensionsPage.js
index c3cb85b07..112557871 100644
--- a/framework/core/js/src/admin/components/ExtensionsPage.js
+++ b/framework/core/js/src/admin/components/ExtensionsPage.js
@@ -1,13 +1,10 @@
-import Page from './Page';
-import LinkButton from '../../common/components/LinkButton';
+import Page from '../../common/components/Page';
 import Button from '../../common/components/Button';
 import Dropdown from '../../common/components/Dropdown';
-import Separator from '../../common/components/Separator';
 import AddExtensionModal from './AddExtensionModal';
 import LoadingModal from './LoadingModal';
 import ItemList from '../../common/utils/ItemList';
 import icon from '../../common/helpers/icon';
-import listItems from '../../common/helpers/listItems';
 
 export default class ExtensionsPage extends Page {
   view() {
diff --git a/framework/core/js/src/admin/components/MailPage.js b/framework/core/js/src/admin/components/MailPage.js
index afbebc2d7..cbec021b3 100644
--- a/framework/core/js/src/admin/components/MailPage.js
+++ b/framework/core/js/src/admin/components/MailPage.js
@@ -1,4 +1,4 @@
-import Page from './Page';
+import Page from '../../common/components/Page';
 import FieldSet from '../../common/components/FieldSet';
 import Button from '../../common/components/Button';
 import Alert from '../../common/components/Alert';
diff --git a/framework/core/js/src/admin/components/Page.js b/framework/core/js/src/admin/components/Page.js
deleted file mode 100644
index 5cf64c75d..000000000
--- a/framework/core/js/src/admin/components/Page.js
+++ /dev/null
@@ -1,32 +0,0 @@
-import Component from '../../common/Component';
-
-/**
- * The `Page` component
- *
- * @abstract
- */
-export default class Page extends Component {
-  init() {
-    app.previous = app.current;
-    app.current = this;
-
-    app.modal.close();
-
-    /**
-     * A class name to apply to the body while the route is active.
-     *
-     * @type {String}
-     */
-    this.bodyClass = '';
-  }
-
-  config(isInitialized, context) {
-    if (isInitialized) return;
-
-    if (this.bodyClass) {
-      $('#app').addClass(this.bodyClass);
-
-      context.onunload = () => $('#app').removeClass(this.bodyClass);
-    }
-  }
-}
diff --git a/framework/core/js/src/admin/components/PermissionsPage.js b/framework/core/js/src/admin/components/PermissionsPage.js
index 3d540d0b1..0bc6ab1f5 100644
--- a/framework/core/js/src/admin/components/PermissionsPage.js
+++ b/framework/core/js/src/admin/components/PermissionsPage.js
@@ -1,4 +1,4 @@
-import Page from './Page';
+import Page from '../../common/components/Page';
 import GroupBadge from '../../common/components/GroupBadge';
 import EditGroupModal from './EditGroupModal';
 import Group from '../../common/models/Group';
diff --git a/framework/core/js/src/common/Application.js b/framework/core/js/src/common/Application.js
index 871f5797b..3525b4b83 100644
--- a/framework/core/js/src/common/Application.js
+++ b/framework/core/js/src/common/Application.js
@@ -21,6 +21,7 @@ import Post from './models/Post';
 import Group from './models/Group';
 import Notification from './models/Notification';
 import { flattenDeep } from 'lodash-es';
+import PageState from './states/PageState';
 
 /**
  * The `App` class provides a container for an application, as well as various
@@ -115,6 +116,28 @@ export default class Application {
    */
   requestError = null;
 
+  /**
+   * The page the app is currently on.
+   *
+   * This object holds information about the type of page we are currently
+   * visiting, and sometimes additional arbitrary page state that may be
+   * relevant to lower-level components.
+   *
+   * @type {PageState}
+   */
+  current = new PageState(null);
+
+  /**
+   * The page the app was on before the current page.
+   *
+   * Once the application navigates to another page, the object previously
+   * assigned to this.current will be moved to this.previous, while this.current
+   * is re-initialized.
+   *
+   * @type {PageState}
+   */
+  previous = new PageState(null);
+
   data;
 
   title = '';
diff --git a/framework/core/js/src/common/compat.js b/framework/core/js/src/common/compat.js
index 0541734a0..a2e5f3d4b 100644
--- a/framework/core/js/src/common/compat.js
+++ b/framework/core/js/src/common/compat.js
@@ -30,6 +30,7 @@ import Forum from './models/Forum';
 import Component from './Component';
 import Translator from './Translator';
 import AlertManager from './components/AlertManager';
+import Page from './components/Page';
 import Switch from './components/Switch';
 import Badge from './components/Badge';
 import LoadingIndicator from './components/LoadingIndicator';
@@ -94,6 +95,7 @@ export default {
   Component: Component,
   Translator: Translator,
   'components/AlertManager': AlertManager,
+  'components/Page': Page,
   'components/Switch': Switch,
   'components/Badge': Badge,
   'components/LoadingIndicator': LoadingIndicator,
diff --git a/framework/core/js/src/common/components/ModalManager.js b/framework/core/js/src/common/components/ModalManager.js
index 4b3a99266..d391631e2 100644
--- a/framework/core/js/src/common/components/ModalManager.js
+++ b/framework/core/js/src/common/components/ModalManager.js
@@ -43,8 +43,6 @@ export default class ModalManager extends Component {
     this.showing = true;
     this.component = component;
 
-    if (app.current) app.current.retain = true;
-
     m.redraw(true);
 
     const dismissible = !!this.component.isDismissible();
diff --git a/framework/core/js/src/forum/components/Page.js b/framework/core/js/src/common/components/Page.js
similarity index 80%
rename from framework/core/js/src/forum/components/Page.js
rename to framework/core/js/src/common/components/Page.js
index 7ef2956cd..fa13c8c20 100644
--- a/framework/core/js/src/forum/components/Page.js
+++ b/framework/core/js/src/common/components/Page.js
@@ -1,4 +1,5 @@
-import Component from '../../common/Component';
+import Component from '../Component';
+import PageState from '../states/PageState';
 
 /**
  * The `Page` component
@@ -8,7 +9,7 @@ import Component from '../../common/Component';
 export default class Page extends Component {
   init() {
     app.previous = app.current;
-    app.current = this;
+    app.current = new PageState(this.constructor);
 
     app.drawer.hide();
     app.modal.close();
diff --git a/framework/core/js/src/common/states/PageState.js b/framework/core/js/src/common/states/PageState.js
new file mode 100644
index 000000000..f8d01301b
--- /dev/null
+++ b/framework/core/js/src/common/states/PageState.js
@@ -0,0 +1,33 @@
+import subclassOf from '../../common/utils/subclassOf';
+
+export default class PageState {
+  constructor(type, data = {}) {
+    this.type = type;
+    this.data = data;
+  }
+
+  /**
+   * Determine whether the page matches the given class and data.
+   *
+   * @param {object} type The page class to check against. Subclasses are
+   *                      accepted as well.
+   * @param {object} data
+   * @return {boolean}
+   */
+  matches(type, data = {}) {
+    // Fail early when the page is of a different type
+    if (!subclassOf(this.type, type)) return false;
+
+    // Now that the type is known to be correct, we loop through the provided
+    // data to see whether it matches the data in our state.
+    return Object.keys(data).every((key) => this.data[key] === data[key]);
+  }
+
+  get(key) {
+    return this.data[key];
+  }
+
+  set(key, value) {
+    this.data[key] = value;
+  }
+}
diff --git a/framework/core/js/src/common/utils/subclassOf.js b/framework/core/js/src/common/utils/subclassOf.js
new file mode 100644
index 000000000..7b4ac2bd0
--- /dev/null
+++ b/framework/core/js/src/common/utils/subclassOf.js
@@ -0,0 +1,6 @@
+/**
+ * Check if class A is the same as or a subclass of class B.
+ */
+export default function subclassOf(A, B) {
+  return A && (A === B || A.prototype instanceof B);
+}
diff --git a/framework/core/js/src/forum/ForumApplication.js b/framework/core/js/src/forum/ForumApplication.js
index a07e91906..78f25dacc 100644
--- a/framework/core/js/src/forum/ForumApplication.js
+++ b/framework/core/js/src/forum/ForumApplication.js
@@ -160,7 +160,7 @@ export default class ForumApplication extends Application {
    * @return {Boolean}
    */
   viewingDiscussion(discussion) {
-    return this.current instanceof DiscussionPage && this.current.discussion === discussion;
+    return this.current.matches(DiscussionPage, { discussion });
   }
 
   /**
diff --git a/framework/core/js/src/forum/compat.js b/framework/core/js/src/forum/compat.js
index ec5c559c7..5d3c439e2 100644
--- a/framework/core/js/src/forum/compat.js
+++ b/framework/core/js/src/forum/compat.js
@@ -23,7 +23,6 @@ import PostEdited from './components/PostEdited';
 import PostStream from './components/PostStream';
 import ChangePasswordModal from './components/ChangePasswordModal';
 import IndexPage from './components/IndexPage';
-import Page from './components/Page';
 import DiscussionRenamedNotification from './components/DiscussionRenamedNotification';
 import DiscussionsSearchSource from './components/DiscussionsSearchSource';
 import HeaderSecondary from './components/HeaderSecondary';
@@ -92,7 +91,6 @@ export default Object.assign(compat, {
   'components/PostStream': PostStream,
   'components/ChangePasswordModal': ChangePasswordModal,
   'components/IndexPage': IndexPage,
-  'components/Page': Page,
   'components/DiscussionRenamedNotification': DiscussionRenamedNotification,
   'components/DiscussionsSearchSource': DiscussionsSearchSource,
   'components/HeaderSecondary': HeaderSecondary,
diff --git a/framework/core/js/src/forum/components/DiscussionPage.js b/framework/core/js/src/forum/components/DiscussionPage.js
index 7c1b48618..27cecc520 100644
--- a/framework/core/js/src/forum/components/DiscussionPage.js
+++ b/framework/core/js/src/forum/components/DiscussionPage.js
@@ -1,4 +1,4 @@
-import Page from './Page';
+import Page from '../../common/components/Page';
 import ItemList from '../../common/utils/ItemList';
 import DiscussionHero from './DiscussionHero';
 import PostStream from './PostStream';
@@ -42,7 +42,7 @@ export default class DiscussionPage extends Page {
       app.pane.enable();
       app.pane.hide();
 
-      if (app.previous instanceof DiscussionPage) {
+      if (app.previous.matches(DiscussionPage)) {
         m.redraw.strategy('diff');
       }
     }
@@ -200,6 +200,9 @@ export default class DiscussionPage extends Page {
     this.stream = new PostStream({ discussion, includedPosts });
     this.stream.on('positionChanged', this.positionChanged.bind(this));
     this.stream.goToNumber(m.route.param('near') || (includedPosts[0] && includedPosts[0].number()), true);
+
+    app.current.set('discussion', discussion);
+    app.current.set('stream', this.stream);
   }
 
   /**
diff --git a/framework/core/js/src/forum/components/IndexPage.js b/framework/core/js/src/forum/components/IndexPage.js
index ee692083f..dc6794b4e 100644
--- a/framework/core/js/src/forum/components/IndexPage.js
+++ b/framework/core/js/src/forum/components/IndexPage.js
@@ -1,5 +1,5 @@
 import { extend } from '../../common/extend';
-import Page from './Page';
+import Page from '../../common/components/Page';
 import ItemList from '../../common/utils/ItemList';
 import listItems from '../../common/helpers/listItems';
 import DiscussionList from './DiscussionList';
@@ -25,15 +25,15 @@ export default class IndexPage extends Page {
     // If the user is returning from a discussion page, then take note of which
     // discussion they have just visited. After the view is rendered, we will
     // scroll down so that this discussion is in view.
-    if (app.previous instanceof DiscussionPage) {
-      this.lastDiscussion = app.previous.discussion;
+    if (app.previous.matches(DiscussionPage)) {
+      this.lastDiscussion = app.previous.get('discussion');
     }
 
     // If the user is coming from the discussion list, then they have either
     // just switched one of the parameters (filter, sort, search) or they
     // probably want to refresh the results. We will clear the discussion list
     // cache so that results are reloaded.
-    if (app.previous instanceof IndexPage) {
+    if (app.previous.matches(IndexPage)) {
       app.discussions.clear();
     }
 
diff --git a/framework/core/js/src/forum/components/NotificationsPage.js b/framework/core/js/src/forum/components/NotificationsPage.js
index 4d8818e18..484e390b3 100644
--- a/framework/core/js/src/forum/components/NotificationsPage.js
+++ b/framework/core/js/src/forum/components/NotificationsPage.js
@@ -1,4 +1,4 @@
-import Page from './Page';
+import Page from '../../common/components/Page';
 import NotificationList from './NotificationList';
 
 /**
diff --git a/framework/core/js/src/forum/components/Post.js b/framework/core/js/src/forum/components/Post.js
index 4843072c9..78bd69dd6 100644
--- a/framework/core/js/src/forum/components/Post.js
+++ b/framework/core/js/src/forum/components/Post.js
@@ -116,6 +116,7 @@ export default class Post extends Component {
     let classes = (existing || '').split(' ').concat(['Post']);
 
     const user = this.props.post.user();
+    const discussion = this.props.post.discussion();
 
     if (this.loading) {
       classes.push('Post--loading');
@@ -125,7 +126,7 @@ export default class Post extends Component {
       classes.push('Post--by-actor');
     }
 
-    if (user && app.current.discussion && app.current.discussion.attribute('startUserId') == user.id()) {
+    if (user && user === discussion.user()) {
       classes.push('Post--by-start-user');
     }
 
diff --git a/framework/core/js/src/forum/components/RenameDiscussionModal.js b/framework/core/js/src/forum/components/RenameDiscussionModal.js
index 5e48f7c57..966e861e3 100644
--- a/framework/core/js/src/forum/components/RenameDiscussionModal.js
+++ b/framework/core/js/src/forum/components/RenameDiscussionModal.js
@@ -57,7 +57,7 @@ export default class RenameDiscussionModal extends Modal {
         .save({ title })
         .then(() => {
           if (app.viewingDiscussion(this.discussion)) {
-            app.current.stream.update();
+            app.current.get('stream').update();
           }
           m.redraw();
           this.hide();
diff --git a/framework/core/js/src/forum/components/ReplyComposer.js b/framework/core/js/src/forum/components/ReplyComposer.js
index ded948182..fd6de3c18 100644
--- a/framework/core/js/src/forum/components/ReplyComposer.js
+++ b/framework/core/js/src/forum/components/ReplyComposer.js
@@ -89,7 +89,8 @@ export default class ReplyComposer extends ComposerBody {
         // If we're currently viewing the discussion which this reply was made
         // in, then we can update the post stream and scroll to the post.
         if (app.viewingDiscussion(discussion)) {
-          app.current.stream.update().then(() => app.current.stream.goToNumber(post.number()));
+          const stream = app.current.get('stream');
+          stream.update().then(() => stream.goToNumber(post.number()));
         } else {
           // Otherwise, we'll create an alert message to inform the user that
           // their reply has been posted, containing a button which will
diff --git a/framework/core/js/src/forum/components/UserPage.js b/framework/core/js/src/forum/components/UserPage.js
index 5569459c8..6793bdf8d 100644
--- a/framework/core/js/src/forum/components/UserPage.js
+++ b/framework/core/js/src/forum/components/UserPage.js
@@ -1,4 +1,4 @@
-import Page from './Page';
+import Page from '../../common/components/Page';
 import ItemList from '../../common/utils/ItemList';
 import affixSidebar from '../utils/affixSidebar';
 import UserCard from './UserCard';
@@ -71,6 +71,8 @@ export default class UserPage extends Page {
   show(user) {
     this.user = user;
 
+    app.current.set('user', user);
+
     app.setTitle(user.displayName());
 
     m.redraw();
diff --git a/framework/core/js/src/forum/states/GlobalSearchState.js b/framework/core/js/src/forum/states/GlobalSearchState.js
index ec3f9538a..6afa69a09 100644
--- a/framework/core/js/src/forum/states/GlobalSearchState.js
+++ b/framework/core/js/src/forum/states/GlobalSearchState.js
@@ -77,7 +77,7 @@ export default class GlobalSearchState extends SearchState {
    * @return {String}
    */
   getInitialSearch() {
-    return app.current.constructor.providesInitialSearch && this.params().q;
+    return app.current.type.providesInitialSearch && this.params().q;
   }
 
   /**
diff --git a/framework/core/js/src/forum/utils/DiscussionControls.js b/framework/core/js/src/forum/utils/DiscussionControls.js
index ebdf7e9d0..862379cc2 100644
--- a/framework/core/js/src/forum/utils/DiscussionControls.js
+++ b/framework/core/js/src/forum/utils/DiscussionControls.js
@@ -178,7 +178,7 @@ export default {
         app.composer.show();
 
         if (goToLast && app.viewingDiscussion(this) && !app.composer.isFullScreen()) {
-          app.current.stream.goToNumber('reply');
+          app.current.get('stream').goToNumber('reply');
         }
 
         deferred.resolve(component);
diff --git a/framework/core/js/src/forum/utils/UserControls.js b/framework/core/js/src/forum/utils/UserControls.js
index 772f127bd..ae1863213 100644
--- a/framework/core/js/src/forum/utils/UserControls.js
+++ b/framework/core/js/src/forum/utils/UserControls.js
@@ -112,7 +112,7 @@ export default {
       .delete()
       .then(() => {
         this.showDeletionAlert(user, 'success');
-        if (app.current instanceof UserPage && app.current.user === user) {
+        if (app.current.matches(UserPage, { user })) {
           app.history.back();
         } else {
           window.location.reload();