Don't store checkbox instances in NotificationGrid (#2183)

* Don't store checkbox states in NotificaitonGrid, use props for loading in Checkbox and Switch, replace preferenceSaver with internal management of loading state

framework/core/js/src/common/components/Checkbox.js

@@ -10,23 +10,14 @@ import icon from '../helpers/icon';
  * - `state` Whether or not the checkbox is checked.
  * - `className` The class name for the root element.
  * - `disabled` Whether or not the checkbox is disabled.
+ * - `loading` Whether or not the checkbox is loading.
  * - `onchange` A callback to run when the checkbox is checked/unchecked.
  * - `children` A text label to display next to the checkbox.
  */
 export default class Checkbox extends Component {
-  init() {
-    /**
-     * Whether or not the checkbox's value is in the process of being saved.
-     *
-     * @type {Boolean}
-     * @public
-     */
-    this.loading = false;
-  }
-
   view() {
     let className = 'Checkbox ' + (this.props.state ? 'on' : 'off') + ' ' + (this.props.className || '');
-    if (this.loading) className += ' loading';
+    if (this.props.loading) className += ' loading';
     if (this.props.disabled) className += ' disabled';
 
     return (
@@ -45,7 +36,7 @@ export default class Checkbox extends Component {
    * @protected
    */
   getDisplay() {
-    return this.loading ? LoadingIndicator.component({ size: 'tiny' }) : icon(this.props.state ? 'fas fa-check' : 'fas fa-times');
+    return this.props.loading ? LoadingIndicator.component({ size: 'tiny' }) : icon(this.props.state ? 'fas fa-check' : 'fas fa-times');
   }
 
   /**

framework/core/js/src/common/components/Switch.js

@@ -12,6 +12,6 @@ export default class Switch extends Checkbox {
   }
 
   getDisplay() {
-    return this.loading ? super.getDisplay() : '';
+    return this.props.loading ? super.getDisplay() : '';
   }
 }

framework/core/js/src/forum/components/NotificationGrid.js

@@ -21,12 +21,11 @@ export default class NotificationGrid extends Component {
     this.methods = this.notificationMethods().toArray();
 
     /**
-     * A map of notification type-method combinations to the checkbox instances
-     * that represent them.
+     * A map of which notification checkboxes are loading.
      *
      * @type {Object}
      */
-    this.inputs = {};
+    this.loading = {};
 
     /**
      * Information about the available notification types.
@@ -34,24 +33,11 @@ export default class NotificationGrid extends Component {
      * @type {Array}
      */
     this.types = this.notificationTypes().toArray();
-
-    // For each of the notification type-method combinations, create and store a
-    // new checkbox component instance, which we will render in the view.
-    this.types.forEach((type) => {
-      this.methods.forEach((method) => {
-        const key = this.preferenceKey(type.name, method.name);
-        const preference = this.props.user.preferences()[key];
-
-        this.inputs[key] = new Checkbox({
-          state: !!preference,
-          disabled: typeof preference === 'undefined',
-          onchange: () => this.toggle([key]),
-        });
-      });
-    });
   }
 
   view() {
+    const preferences = this.props.user.preferences();
+
     return (
       <table className="NotificationGrid">
         <thead>
@@ -71,9 +57,20 @@ export default class NotificationGrid extends Component {
               <td className="NotificationGrid-groupToggle" onclick={this.toggleType.bind(this, type.name)}>
                 {icon(type.icon)} {type.label}
               </td>
-              {this.methods.map((method) => (
-                <td className="NotificationGrid-checkbox">{this.inputs[this.preferenceKey(type.name, method.name)].render()}</td>
-              ))}
+              {this.methods.map((method) => {
+                const key = this.preferenceKey(type.name, method.name);
+
+                return (
+                  <td className="NotificationGrid-checkbox">
+                    {Checkbox.component({
+                      state: !!preferences[key],
+                      loading: this.loading[key],
+                      disabled: !(key in preferences),
+                      onchange: () => this.toggle([key]),
+                    })}
+                  </td>
+                );
+              })}
             </tr>
           ))}
         </tbody>
@@ -112,16 +109,14 @@ export default class NotificationGrid extends Component {
     const enabled = !preferences[keys[0]];
 
     keys.forEach((key) => {
-      const control = this.inputs[key];
-
-      control.loading = true;
-      preferences[key] = control.props.state = enabled;
+      this.loading[key] = true;
+      preferences[key] = enabled;
     });
 
     m.redraw();
 
     user.save({ preferences }).then(() => {
-      keys.forEach((key) => (this.inputs[key].loading = false));
+      keys.forEach((key) => (this.loading[key] = false));
 
       m.redraw();
     });
@@ -133,7 +128,7 @@ export default class NotificationGrid extends Component {
    * @param {String} method
    */
   toggleMethod(method) {
-    const keys = this.types.map((type) => this.preferenceKey(type.name, method)).filter((key) => !this.inputs[key].props.disabled);
+    const keys = this.types.map((type) => this.preferenceKey(type.name, method)).filter((key) => key in this.props.user.preferences());
 
     this.toggle(keys);
   }
@@ -144,7 +139,7 @@ export default class NotificationGrid extends Component {
    * @param {String} type
    */
   toggleType(type) {
-    const keys = this.methods.map((method) => this.preferenceKey(type, method.name)).filter((key) => !this.inputs[key].props.disabled);
+    const keys = this.methods.map((method) => this.preferenceKey(type, method.name)).filter((key) => key in this.props.user.preferences());
 
     this.toggle(keys);
   }

framework/core/js/src/forum/components/SettingsPage.js

@@ -109,6 +109,8 @@ export default class SettingsPage extends UserPage {
   }
 
   /**
+   * @deprecated beta 14, remove in beta 15.
+   *
    * Generate a callback that will save a value to the given preference.
    *
    * @param {String} key
@@ -116,11 +118,11 @@ export default class SettingsPage extends UserPage {
    */
   preferenceSaver(key) {
     return (value, component) => {
-      if (component) component.loading = true;
+      if (component) component.props.loading = true;
       m.redraw();
 
       this.user.savePreferences({ [key]: value }).then(() => {
-        if (component) component.loading = false;
+        if (component) component.props.loading = false;
         m.redraw();
       });
     };
@@ -139,10 +141,15 @@ export default class SettingsPage extends UserPage {
       Switch.component({
         children: app.translator.trans('core.forum.settings.privacy_disclose_online_label'),
         state: this.user.preferences().discloseOnline,
-        onchange: (value, component) => {
-          this.user.pushAttributes({ lastSeenAt: null });
-          this.preferenceSaver('discloseOnline')(value, component);
+        onchange: (value) => {
+          this.discloseOnlineLoading = true;
+
+          this.user.savePreferences({ discloseOnline: value }).then(() => {
+            this.discloseOnlineLoading = false;
+            m.redraw();
+          });
         },
+        loading: this.discloseOnlineLoading,
       })
     );