issue/3105-react-fixes various improvements (#3140)

.eslintrc.json

@@ -6,11 +6,10 @@
         "amd": true
     },
     "extends": [
-        "standard"
+        "standard",
+        "plugin:react/recommended"
     ],
     "globals": {
-        "ReactDOM": true,
-        "React": true,
         "Promise": true,
         "requirejs": true,
         "Backbone": true,
@@ -23,13 +22,17 @@
         "SharedArrayBuffer": "readonly"
     },
     "parserOptions": {
-        "ecmaVersion": 2020
+        "ecmaVersion": 2020,
+        "ecmaFeatures": {
+            "jsx": true
+        }
     },
     "plugins": [
         "requirejs"
     ],
     "rules": {
         "indent": ["error", 2, { "SwitchCase": 1 }],
+        "array-bracket-spacing": "off",
         "semi": ["error", "always"],
         "padded-blocks": "off",
         "no-new": "off",
@@ -39,7 +42,8 @@
         "requirejs/no-multiple-define": 2,
         "requirejs/no-named-define": "off",
         "requirejs/no-commonjs-wrapper": 2,
-        "requirejs/no-object-define": 1
+        "requirejs/no-object-define": 1,
+        "react/prop-types": "off"
     },
     "settings": {
         "react": {

package.json

@@ -57,13 +57,14 @@
         "underscore-deep-extend": "^1.1.5"
     },
     "devDependencies": {
-        "eslint": "^6.0.1",
-        "eslint-config-standard": "^12.0.0",
-        "eslint-plugin-import": "^2.18.0",
-        "eslint-plugin-node": "^9.1.0",
-        "eslint-plugin-promise": "^4.2.1",
-        "eslint-plugin-requirejs": "^4.0.0",
-        "eslint-plugin-standard": "^4.0.0"
+        "eslint": "^7.25.0",
+        "eslint-config-standard": "^16.0.2",
+        "eslint-plugin-import": "^2.22.1",
+        "eslint-plugin-node": "^11.1.0",
+        "eslint-plugin-promise": "^5.1.0",
+        "eslint-plugin-react": "^7.23.2",
+        "eslint-plugin-requirejs": "^4.0.1",
+        "eslint-plugin-standard": "^5.0.0"
     },
     "optionalDependencies": {
         "imagemin": "^7.0.1",

src/core/js/fixes/img.lazyload.js

@@ -1,6 +1,5 @@
 import Adapt from 'core/js/adapt';
 import 'core/js/templates';
-import { find, clone } from 'core/js/reactHelpers';
 
 /**
  * 27 April 2020 https://github.com/adaptlearning/adapt_framework/issues/2734
@@ -32,18 +31,10 @@ function applyImgLoadingFix() {
       return value.replace(img, img.replace(findImgTag, '<img loading="eager"$1>'));
     }, event.value);
   });
-  Adapt.on('reactTemplate:postRender', function(event) {
-    const hasImageTagWithNoLoadingAttr = find(event.value, component => {
-      if (component.type !== 'img') return;
-      if (component.props.loading) return;
-      return true;
-    });
-    if (!hasImageTagWithNoLoadingAttr) return;
-    // Strip object freeze and write locks by cloning
-    event.value = clone(event.value, true, component => {
-      if (component.type !== 'img') return;
-      if (component.props.loading) return;
-      component.props.loading = 'eager';
-    });
+  Adapt.on('reactElement:preRender', event => {
+    if (event.name !== 'img') return;
+    const options = event.args[1] = event.args[1] || {};
+    if (options && options.hasOwnProperty('loading')) return;
+    options.loading = 'eager';
   });
 }

src/core/js/reactHelpers.js

@@ -1,63 +1,7 @@
 import Adapt from 'core/js/adapt';
 import TemplateRenderEvent from './templateRenderEvent';
 import HTMLReactParser from 'html-react-parser';
-
-/**
- * Finds a node in a react node hierarchy
- * Return true from the iterator to stop traversal
- * @param {object} hierarchy
- * @param {function} iterator
- */
-export function find(hierarchy, iterator) {
-  if (iterator(hierarchy)) {
-    return true;
-  }
-  if (!hierarchy.props || !hierarchy.props.children) return;
-  if (Array.isArray(hierarchy.props.children)) {
-    return hierarchy.props.children.find(child => {
-      if (!child) return;
-      return find(child, iterator);
-    });
-  }
-  return find(hierarchy.props.children, iterator);
-};
-
-/**
- * Allows clone and modification of a react node hierarchy
- * @param {*} value
- * @param {boolean} isDeep=false
- * @param {function} modifier
- * @returns {*}
- */
-export function clone(value, isDeep = false, modifier = null) {
-  if (typeof value !== 'object' || value === null) {
-    return value;
-  }
-  const cloned = Array.isArray(value) ? [] : {};
-  const descriptors = Object.getOwnPropertyDescriptors(value);
-  for (let name in descriptors) {
-    const descriptor = descriptors[name];
-    if (!descriptor.hasOwnProperty('value')) {
-      Object.defineProperty(cloned, name, descriptor);
-      continue;
-    }
-    let value = descriptor.value;
-    if (typeof value === 'object' && value !== null) {
-      if (isDeep) {
-        value = descriptor.value = clone(value, isDeep, modifier);
-      }
-      if (modifier && typeof value.$$typeof === 'symbol') {
-        modifier(value);
-      }
-    }
-    descriptor.writable = true;
-    Object.defineProperty(cloned, name, descriptor);
-  }
-  if (modifier && typeof cloned.$$typeof === 'symbol') {
-    modifier(cloned);
-  }
-  return cloned;
-};
+import React from 'react';
 
 /**
  * Used by babel plugin babel-plugin-transform-react-templates to inject react templates
@@ -79,6 +23,29 @@ export default function register(name, component) {
   };
 };
 
+/**
+ * Override React.createElement to allow trapping and modification of react
+ * template elements.
+ */
+(function () {
+  const original = React.createElement;
+  React.createElement = (...args) => {
+    const name = args[0];
+    // Trap render calls to emit preRender and postRender events
+    const mode = 'reactElement';
+    // Send preRender event to allow modification of args
+    const preRenderEvent = new TemplateRenderEvent(`${mode}:preRender`, name, mode, null, args);
+    Adapt.trigger(preRenderEvent.type, preRenderEvent);
+    // Execute element creation
+    const value = original(...preRenderEvent.args);
+    // Send postRender event to allow modification of rendered element
+    const postRenderEvent = new TemplateRenderEvent(`${mode}:postRender`, name, mode, value, preRenderEvent.args);
+    Adapt.trigger(postRenderEvent.type, postRenderEvent);
+    // Return rendered, modified element
+    return postRenderEvent.value;
+  };
+})();
+
 /**
  * Storage for react templates
  */
@@ -92,24 +59,12 @@ export function html(html, ref = null) {
   if (!html) return;
   let node = html ? HTMLReactParser(html) : '';
   if (typeof node === 'object' && ref) {
-    // Strip object freeze and write locks by cloning
-    node = clone(node);
-    node.ref = ref;
+    node = Array.isArray(node) ? node[0] : node;
+    node = React.cloneElement(node, { ref });
   }
   return node;
 }
 
-/**
- * Render the named react component
- * @param {string} name React template name
- * @param {...any} args React template arguments
- */
-export function render(name, ...args) {
-  const template = templates[name];
-  const component = template(...args);
-  return component;
-};
-
 /**
  * Handlebars compile integration
  * @param {string} name Handlebars template
@@ -141,9 +96,20 @@ export function helper(name, ...args) {
 };
 
 /**
- * Helper for a list of classes, filtering out falsies and joining with spaces
+ * Helper for a list of classes, filtering out falsies and duplicates, and joining with spaces
  * @param  {...any} args List or arrays of classes
  */
 export function classes(...args) {
-  return _.flatten(args).filter(Boolean).join(' ');
+  return _.uniq(_.flatten(args).filter(Boolean).join(' ').split(' ')).join(' ');
+};
+
+/**
+ * Helper for prefixing a list of classes, filtering out falsies and duplicates and joining with spaces
+ * @param  {[...string]} prefixes Array of class prefixes
+ * @param  {...any} args List or arrays of classes
+ */
+export function prefixClasses(prefixes, ...args) {
+  const classes = _.flatten(args).filter(Boolean);
+  const prefixed = _.flatten(prefixes.map(prefix => classes.map(className => `${prefix}${className}`)));
+  return _.uniq(prefixed.join(' ').split(' ')).join(' ');
 };

src/core/js/views/adaptView.js

@@ -1,6 +1,7 @@
 import Adapt from 'core/js/adapt';
 import ChildEvent from 'core/js/childEvent';
-import { render } from 'core/js/reactHelpers';
+import { templates } from 'core/js/reactHelpers';
+import React from 'react';
 import ReactDOM from 'react-dom';
 
 class AdaptView extends Backbone.View {
@@ -17,16 +18,18 @@ class AdaptView extends Backbone.View {
       'change:_isHidden': this.toggleHidden,
       'change:_isComplete': this.onIsCompleteChange
     });
-    this.isReact = (this.constructor.template || '').includes('.jsx');
-    if (this.isReact) {
+    this.isJSX = (this.constructor.template || '').includes('.jsx');
+    if (this.isJSX) {
       this._classSet = new Set(_.result(this, 'className').trim().split(/\s+/));
       this.listenTo(this.model, 'all', this.changed);
+      const children = this.model?.getChildren?.();
+      children && this.listenTo(children, 'all', this.changed);
       // Facilitate adaptive react views
       this.listenTo(Adapt, 'device:changed', this.changed);
     }
     this.model.set({
-      '_globals': Adapt.course.get('_globals'),
-      '_isReady': false
+      _globals: Adapt.course.get('_globals'),
+      _isReady: false
     });
     this._isRemoved = false;
 
@@ -49,7 +52,7 @@ class AdaptView extends Backbone.View {
     const type = this.constructor.type;
     Adapt.trigger(`${type}View:preRender view:preRender`, this);
 
-    if (this.isReact) {
+    if (this.isJSX) {
       this.changed();
     } else {
       const data = this.model.toJSON();
@@ -76,16 +79,24 @@ class AdaptView extends Backbone.View {
    * @param {string} eventName=null Backbone change event name
    */
   changed(eventName = null) {
-    if (!this.isReact) {
+    if (!this.isJSX) {
       throw new Error('Cannot call changed on a non-react view');
     }
     if (typeof eventName === 'string' && eventName.startsWith('bubble')) {
       // Ignore bubbling events as they are outside of this view's scope
       return;
     }
-    const element = render(this.constructor.template.replace('.jsx', ''), this.model, this);
+    const props = {
+      // Add view own properties, bound functions etc
+      ...this,
+      // Add model json data
+      ...this.model.toJSON(),
+      // Add globals
+      _globals: Adapt.course.get('_globals')
+    };
+    const Template = templates[this.constructor.template.replace('.jsx', '')];
     this.updateViewProperties();
-    ReactDOM.render(element, this.el);
+    ReactDOM.render(<Template {...props} />, this.el);
   }
 
   updateViewProperties() {
@@ -143,8 +154,8 @@ class AdaptView extends Backbone.View {
       // Set model state
       const model = event.model;
       model.set({
-        '_isRendered': true,
-        '_nthChild': ++this.nthChild
+        _isRendered: true,
+        _nthChild: ++this.nthChild
       });
       // Create child view
       const ChildView = this.constructor.childView || Adapt.getViewClass(model);
@@ -248,7 +259,7 @@ class AdaptView extends Backbone.View {
    */
   _getAddChildEvent(model) {
     const isRequestChild = (!model);
-    let event = new ChildEvent(null, this, model);
+    const event = new ChildEvent(null, this, model);
     if (isRequestChild) {
       // No model has been supplied, we are at the end of the available child list
       const canRequestChild = this.model.get('_canRequestChild');
@@ -328,7 +339,7 @@ class AdaptView extends Backbone.View {
     this.stopListening();
 
     Adapt.wait.for(end => {
-      if (this.isReact) {
+      if (this.isJSX) {
         ReactDOM.unmountComponentAtNode(this.el);
       }
       this.$el.off('onscreen.adaptView');
@@ -384,7 +395,7 @@ class AdaptView extends Backbone.View {
    * @returns {{<string, AdaptView}}
    */
   get childViews() {
-    Adapt.log.deprecated(`view.childViews use view.getChildViews() and view.setChildViews([])`);
+    Adapt.log.deprecated('view.childViews use view.getChildViews() and view.setChildViews([])');
     if (Array.isArray(this._childViews)) {
       return _.indexBy(this._childViews, view => view.model.get('_id'));
     }
@@ -396,7 +407,7 @@ class AdaptView extends Backbone.View {
    * @deprecated since 5.7.0
    */
   set childViews(value) {
-    Adapt.log.deprecated(`view.childViews use view.getChildViews() and view.setChildViews([])`);
+    Adapt.log.deprecated('view.childViews use view.getChildViews() and view.setChildViews([])');
     this.setChildViews(value);
   }
 

src/core/js/views/questionView.js

@@ -24,6 +24,20 @@ class QuestionView extends ComponentView {
     ].join(' ');
   }
 
+  initialize(...args) {
+    // Allow isInteractive to be used in jsx templates
+    this.isInteractive = this.isInteractive.bind(this);
+    super.initialize(...args);
+  }
+
+  /**
+   * Used to determine whether the learner is allowed to interact with the question component or not.
+   * @return {Boolean}
+  */
+  isInteractive() {
+    return this.model.isInteractive();
+  }
+
   preRender() {
     // Setup listener for _isEnabled
     this.listenTo(this.model, 'change:_isEnabled', this.onEnabledChanged);