Skip to content

Commit 208f454

Browse files
authored
issue/3105-react-fixes various improvements (#3140)
1 parent 11ccf0c commit 208f454

File tree

9 files changed

+220
-180
lines changed

9 files changed

+220
-180
lines changed

.eslintrc.json

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,10 @@
66
"amd": true
77
},
88
"extends": [
9-
"standard"
9+
"standard",
10+
"plugin:react/recommended"
1011
],
1112
"globals": {
12-
"ReactDOM": true,
13-
"React": true,
1413
"Promise": true,
1514
"requirejs": true,
1615
"Backbone": true,
@@ -23,13 +22,17 @@
2322
"SharedArrayBuffer": "readonly"
2423
},
2524
"parserOptions": {
26-
"ecmaVersion": 2020
25+
"ecmaVersion": 2020,
26+
"ecmaFeatures": {
27+
"jsx": true
28+
}
2729
},
2830
"plugins": [
2931
"requirejs"
3032
],
3133
"rules": {
3234
"indent": ["error", 2, { "SwitchCase": 1 }],
35+
"array-bracket-spacing": "off",
3336
"semi": ["error", "always"],
3437
"padded-blocks": "off",
3538
"no-new": "off",
@@ -39,7 +42,8 @@
3942
"requirejs/no-multiple-define": 2,
4043
"requirejs/no-named-define": "off",
4144
"requirejs/no-commonjs-wrapper": 2,
42-
"requirejs/no-object-define": 1
45+
"requirejs/no-object-define": 1,
46+
"react/prop-types": "off"
4347
},
4448
"settings": {
4549
"react": {

package.json

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,14 @@
5757
"underscore-deep-extend": "^1.1.5"
5858
},
5959
"devDependencies": {
60-
"eslint": "^6.0.1",
61-
"eslint-config-standard": "^12.0.0",
62-
"eslint-plugin-import": "^2.18.0",
63-
"eslint-plugin-node": "^9.1.0",
64-
"eslint-plugin-promise": "^4.2.1",
65-
"eslint-plugin-requirejs": "^4.0.0",
66-
"eslint-plugin-standard": "^4.0.0"
60+
"eslint": "^7.25.0",
61+
"eslint-config-standard": "^16.0.2",
62+
"eslint-plugin-import": "^2.22.1",
63+
"eslint-plugin-node": "^11.1.0",
64+
"eslint-plugin-promise": "^5.1.0",
65+
"eslint-plugin-react": "^7.23.2",
66+
"eslint-plugin-requirejs": "^4.0.1",
67+
"eslint-plugin-standard": "^5.0.0"
6768
},
6869
"optionalDependencies": {
6970
"imagemin": "^7.0.1",

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

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import Adapt from 'core/js/adapt';
22
import 'core/js/templates';
3-
import { find, clone } from 'core/js/reactHelpers';
43

54
/**
65
* 27 April 2020 https://github.com/adaptlearning/adapt_framework/issues/2734
@@ -32,18 +31,10 @@ function applyImgLoadingFix() {
3231
return value.replace(img, img.replace(findImgTag, '<img loading="eager"$1>'));
3332
}, event.value);
3433
});
35-
Adapt.on('reactTemplate:postRender', function(event) {
36-
const hasImageTagWithNoLoadingAttr = find(event.value, component => {
37-
if (component.type !== 'img') return;
38-
if (component.props.loading) return;
39-
return true;
40-
});
41-
if (!hasImageTagWithNoLoadingAttr) return;
42-
// Strip object freeze and write locks by cloning
43-
event.value = clone(event.value, true, component => {
44-
if (component.type !== 'img') return;
45-
if (component.props.loading) return;
46-
component.props.loading = 'eager';
47-
});
34+
Adapt.on('reactElement:preRender', event => {
35+
if (event.name !== 'img') return;
36+
const options = event.args[1] = event.args[1] || {};
37+
if (options && options.hasOwnProperty('loading')) return;
38+
options.loading = 'eager';
4839
});
4940
}

src/core/js/reactHelpers.js

Lines changed: 39 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,63 +1,7 @@
11
import Adapt from 'core/js/adapt';
22
import TemplateRenderEvent from './templateRenderEvent';
33
import HTMLReactParser from 'html-react-parser';
4-
5-
/**
6-
* Finds a node in a react node hierarchy
7-
* Return true from the iterator to stop traversal
8-
* @param {object} hierarchy
9-
* @param {function} iterator
10-
*/
11-
export function find(hierarchy, iterator) {
12-
if (iterator(hierarchy)) {
13-
return true;
14-
}
15-
if (!hierarchy.props || !hierarchy.props.children) return;
16-
if (Array.isArray(hierarchy.props.children)) {
17-
return hierarchy.props.children.find(child => {
18-
if (!child) return;
19-
return find(child, iterator);
20-
});
21-
}
22-
return find(hierarchy.props.children, iterator);
23-
};
24-
25-
/**
26-
* Allows clone and modification of a react node hierarchy
27-
* @param {*} value
28-
* @param {boolean} isDeep=false
29-
* @param {function} modifier
30-
* @returns {*}
31-
*/
32-
export function clone(value, isDeep = false, modifier = null) {
33-
if (typeof value !== 'object' || value === null) {
34-
return value;
35-
}
36-
const cloned = Array.isArray(value) ? [] : {};
37-
const descriptors = Object.getOwnPropertyDescriptors(value);
38-
for (let name in descriptors) {
39-
const descriptor = descriptors[name];
40-
if (!descriptor.hasOwnProperty('value')) {
41-
Object.defineProperty(cloned, name, descriptor);
42-
continue;
43-
}
44-
let value = descriptor.value;
45-
if (typeof value === 'object' && value !== null) {
46-
if (isDeep) {
47-
value = descriptor.value = clone(value, isDeep, modifier);
48-
}
49-
if (modifier && typeof value.$$typeof === 'symbol') {
50-
modifier(value);
51-
}
52-
}
53-
descriptor.writable = true;
54-
Object.defineProperty(cloned, name, descriptor);
55-
}
56-
if (modifier && typeof cloned.$$typeof === 'symbol') {
57-
modifier(cloned);
58-
}
59-
return cloned;
60-
};
4+
import React from 'react';
615

626
/**
637
* Used by babel plugin babel-plugin-transform-react-templates to inject react templates
@@ -79,6 +23,29 @@ export default function register(name, component) {
7923
};
8024
};
8125

26+
/**
27+
* Override React.createElement to allow trapping and modification of react
28+
* template elements.
29+
*/
30+
(function () {
31+
const original = React.createElement;
32+
React.createElement = (...args) => {
33+
const name = args[0];
34+
// Trap render calls to emit preRender and postRender events
35+
const mode = 'reactElement';
36+
// Send preRender event to allow modification of args
37+
const preRenderEvent = new TemplateRenderEvent(`${mode}:preRender`, name, mode, null, args);
38+
Adapt.trigger(preRenderEvent.type, preRenderEvent);
39+
// Execute element creation
40+
const value = original(...preRenderEvent.args);
41+
// Send postRender event to allow modification of rendered element
42+
const postRenderEvent = new TemplateRenderEvent(`${mode}:postRender`, name, mode, value, preRenderEvent.args);
43+
Adapt.trigger(postRenderEvent.type, postRenderEvent);
44+
// Return rendered, modified element
45+
return postRenderEvent.value;
46+
};
47+
})();
48+
8249
/**
8350
* Storage for react templates
8451
*/
@@ -92,24 +59,12 @@ export function html(html, ref = null) {
9259
if (!html) return;
9360
let node = html ? HTMLReactParser(html) : '';
9461
if (typeof node === 'object' && ref) {
95-
// Strip object freeze and write locks by cloning
96-
node = clone(node);
97-
node.ref = ref;
62+
node = Array.isArray(node) ? node[0] : node;
63+
node = React.cloneElement(node, { ref });
9864
}
9965
return node;
10066
}
10167

102-
/**
103-
* Render the named react component
104-
* @param {string} name React template name
105-
* @param {...any} args React template arguments
106-
*/
107-
export function render(name, ...args) {
108-
const template = templates[name];
109-
const component = template(...args);
110-
return component;
111-
};
112-
11368
/**
11469
* Handlebars compile integration
11570
* @param {string} name Handlebars template
@@ -141,9 +96,20 @@ export function helper(name, ...args) {
14196
};
14297

14398
/**
144-
* Helper for a list of classes, filtering out falsies and joining with spaces
99+
* Helper for a list of classes, filtering out falsies and duplicates, and joining with spaces
145100
* @param {...any} args List or arrays of classes
146101
*/
147102
export function classes(...args) {
148-
return _.flatten(args).filter(Boolean).join(' ');
103+
return _.uniq(_.flatten(args).filter(Boolean).join(' ').split(' ')).join(' ');
104+
};
105+
106+
/**
107+
* Helper for prefixing a list of classes, filtering out falsies and duplicates and joining with spaces
108+
* @param {[...string]} prefixes Array of class prefixes
109+
* @param {...any} args List or arrays of classes
110+
*/
111+
export function prefixClasses(prefixes, ...args) {
112+
const classes = _.flatten(args).filter(Boolean);
113+
const prefixed = _.flatten(prefixes.map(prefix => classes.map(className => `${prefix}${className}`)));
114+
return _.uniq(prefixed.join(' ').split(' ')).join(' ');
149115
};

src/core/js/views/adaptView.js

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import Adapt from 'core/js/adapt';
22
import ChildEvent from 'core/js/childEvent';
3-
import { render } from 'core/js/reactHelpers';
3+
import { templates } from 'core/js/reactHelpers';
4+
import React from 'react';
45
import ReactDOM from 'react-dom';
56

67
class AdaptView extends Backbone.View {
@@ -17,16 +18,18 @@ class AdaptView extends Backbone.View {
1718
'change:_isHidden': this.toggleHidden,
1819
'change:_isComplete': this.onIsCompleteChange
1920
});
20-
this.isReact = (this.constructor.template || '').includes('.jsx');
21-
if (this.isReact) {
21+
this.isJSX = (this.constructor.template || '').includes('.jsx');
22+
if (this.isJSX) {
2223
this._classSet = new Set(_.result(this, 'className').trim().split(/\s+/));
2324
this.listenTo(this.model, 'all', this.changed);
25+
const children = this.model?.getChildren?.();
26+
children && this.listenTo(children, 'all', this.changed);
2427
// Facilitate adaptive react views
2528
this.listenTo(Adapt, 'device:changed', this.changed);
2629
}
2730
this.model.set({
28-
'_globals': Adapt.course.get('_globals'),
29-
'_isReady': false
31+
_globals: Adapt.course.get('_globals'),
32+
_isReady: false
3033
});
3134
this._isRemoved = false;
3235

@@ -49,7 +52,7 @@ class AdaptView extends Backbone.View {
4952
const type = this.constructor.type;
5053
Adapt.trigger(`${type}View:preRender view:preRender`, this);
5154

52-
if (this.isReact) {
55+
if (this.isJSX) {
5356
this.changed();
5457
} else {
5558
const data = this.model.toJSON();
@@ -76,16 +79,24 @@ class AdaptView extends Backbone.View {
7679
* @param {string} eventName=null Backbone change event name
7780
*/
7881
changed(eventName = null) {
79-
if (!this.isReact) {
82+
if (!this.isJSX) {
8083
throw new Error('Cannot call changed on a non-react view');
8184
}
8285
if (typeof eventName === 'string' && eventName.startsWith('bubble')) {
8386
// Ignore bubbling events as they are outside of this view's scope
8487
return;
8588
}
86-
const element = render(this.constructor.template.replace('.jsx', ''), this.model, this);
89+
const props = {
90+
// Add view own properties, bound functions etc
91+
...this,
92+
// Add model json data
93+
...this.model.toJSON(),
94+
// Add globals
95+
_globals: Adapt.course.get('_globals')
96+
};
97+
const Template = templates[this.constructor.template.replace('.jsx', '')];
8798
this.updateViewProperties();
88-
ReactDOM.render(element, this.el);
99+
ReactDOM.render(<Template {...props} />, this.el);
89100
}
90101

91102
updateViewProperties() {
@@ -143,8 +154,8 @@ class AdaptView extends Backbone.View {
143154
// Set model state
144155
const model = event.model;
145156
model.set({
146-
'_isRendered': true,
147-
'_nthChild': ++this.nthChild
157+
_isRendered: true,
158+
_nthChild: ++this.nthChild
148159
});
149160
// Create child view
150161
const ChildView = this.constructor.childView || Adapt.getViewClass(model);
@@ -248,7 +259,7 @@ class AdaptView extends Backbone.View {
248259
*/
249260
_getAddChildEvent(model) {
250261
const isRequestChild = (!model);
251-
let event = new ChildEvent(null, this, model);
262+
const event = new ChildEvent(null, this, model);
252263
if (isRequestChild) {
253264
// No model has been supplied, we are at the end of the available child list
254265
const canRequestChild = this.model.get('_canRequestChild');
@@ -328,7 +339,7 @@ class AdaptView extends Backbone.View {
328339
this.stopListening();
329340

330341
Adapt.wait.for(end => {
331-
if (this.isReact) {
342+
if (this.isJSX) {
332343
ReactDOM.unmountComponentAtNode(this.el);
333344
}
334345
this.$el.off('onscreen.adaptView');
@@ -384,7 +395,7 @@ class AdaptView extends Backbone.View {
384395
* @returns {{<string, AdaptView}}
385396
*/
386397
get childViews() {
387-
Adapt.log.deprecated(`view.childViews use view.getChildViews() and view.setChildViews([])`);
398+
Adapt.log.deprecated('view.childViews use view.getChildViews() and view.setChildViews([])');
388399
if (Array.isArray(this._childViews)) {
389400
return _.indexBy(this._childViews, view => view.model.get('_id'));
390401
}
@@ -396,7 +407,7 @@ class AdaptView extends Backbone.View {
396407
* @deprecated since 5.7.0
397408
*/
398409
set childViews(value) {
399-
Adapt.log.deprecated(`view.childViews use view.getChildViews() and view.setChildViews([])`);
410+
Adapt.log.deprecated('view.childViews use view.getChildViews() and view.setChildViews([])');
400411
this.setChildViews(value);
401412
}
402413

src/core/js/views/questionView.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,20 @@ class QuestionView extends ComponentView {
2424
].join(' ');
2525
}
2626

27+
initialize(...args) {
28+
// Allow isInteractive to be used in jsx templates
29+
this.isInteractive = this.isInteractive.bind(this);
30+
super.initialize(...args);
31+
}
32+
33+
/**
34+
* Used to determine whether the learner is allowed to interact with the question component or not.
35+
* @return {Boolean}
36+
*/
37+
isInteractive() {
38+
return this.model.isInteractive();
39+
}
40+
2741
preRender() {
2842
// Setup listener for _isEnabled
2943
this.listenTo(this.model, 'change:_isEnabled', this.onEnabledChanged);

0 commit comments

Comments
 (0)