patternjavascriptMinor
Interactive small town
Viewed 0 times
towninteractivesmall
Problem
I've built this simple interactive scenario. Now everything seems to be working fine except for the start of the
Here is the full code: http://codepen.io/Angussimons/pen/oZmNer
```
for (let i = 0; i < manNumber; i++) {
// Definisco quale strada
if (i < (manNumber * leftStreet)) {
street[i] = document.getElementById('path-2');
} else {
street[i] = document.getElementById('path-1');
}
// genero una velocità per ognuno
_speed[i] = Math.floor((Math.random() * 400) + 1);
_speed[i] = (_speed[i]/1000);
let shiftTop = -1.5;
// genero una posizione su ognuno
let manPosition = shiftTop + Math.floor((Math.random() * 1000) - 500)/1000;
// vario la durata dei gesti
var _duration = Math.floor((Math.random() * 1000) + 500);
mansHead.push( new mojs.Shape({
parent: '.omini',
className: 'omino man-head-'+i,
shape: 'circle',
radius: 8,
fill: manColor,
top: manPosition+'%',
left: 0,
x: {0 : 5},
y: 10,
easing: 'linear.none',
duration: _duration,
}).then({
x: {5 : 0},
}));
mansBody.push(new mojs.Shape({
parent: '.man-head-'+i,
className: 'man-body-'+i,
shape: 'line',
fill: 'none',
stroke: manColor,
radius: 10,
strokeWidth: 13,
strokeLinecap: 'round',
angle: {85 : 95},
x: {5 : -2},
y: 30,
easing: 'linear.none',
duration: _duration,
}).then({
angle: {95 : 85},
x: {[-2] : 5},
y: 30,
}));
mansArmL.push(new mojs.Shape({
parent: '.man-body-'+i,
className: 'man-arm-'+i,
shape: 'line',
fill: 'none',
s
for loop as below. What can I do to make the script execution faster?Here is the full code: http://codepen.io/Angussimons/pen/oZmNer
```
for (let i = 0; i < manNumber; i++) {
// Definisco quale strada
if (i < (manNumber * leftStreet)) {
street[i] = document.getElementById('path-2');
} else {
street[i] = document.getElementById('path-1');
}
// genero una velocità per ognuno
_speed[i] = Math.floor((Math.random() * 400) + 1);
_speed[i] = (_speed[i]/1000);
let shiftTop = -1.5;
// genero una posizione su ognuno
let manPosition = shiftTop + Math.floor((Math.random() * 1000) - 500)/1000;
// vario la durata dei gesti
var _duration = Math.floor((Math.random() * 1000) + 500);
mansHead.push( new mojs.Shape({
parent: '.omini',
className: 'omino man-head-'+i,
shape: 'circle',
radius: 8,
fill: manColor,
top: manPosition+'%',
left: 0,
x: {0 : 5},
y: 10,
easing: 'linear.none',
duration: _duration,
}).then({
x: {5 : 0},
}));
mansBody.push(new mojs.Shape({
parent: '.man-head-'+i,
className: 'man-body-'+i,
shape: 'line',
fill: 'none',
stroke: manColor,
radius: 10,
strokeWidth: 13,
strokeLinecap: 'round',
angle: {85 : 95},
x: {5 : -2},
y: 30,
easing: 'linear.none',
duration: _duration,
}).then({
angle: {95 : 85},
x: {[-2] : 5},
y: 30,
}));
mansArmL.push(new mojs.Shape({
parent: '.man-body-'+i,
className: 'man-arm-'+i,
shape: 'line',
fill: 'none',
s
Solution
JavaScript
The loop that generates people - an expensive bridge to cross
”...DOM access is actually pretty costly - I think of it like if I have a bridge - like two pieces of land with a toll bridge, and the JavaScript engine is on one side, and the DOM is on the other, and every time I want to access the DOM from the JavaScript engine, I have to pay that toll”
- John Hrvatin, Microsoft, MIX09, in this talk Building High Performance Web Applications and Sites at 29:38, also cited in the O'Reilly Javascript book by Nicholas C Zakas Pg 36, as well as mentioned in this post
Bearing in mind that was stated more than 10 years ago and browsers likely have come along way since then, it is still wise to be aware of this.
This code block makes quite a few redundant DOM look-ups - 30 (based on the value I see assigned to
It would be more efficient to look up those two elements outside the loop - preferably once the DOM is ready...
The inside that loop, use those references:
Selecting elements to get innerHTML
The function
It isn't wrong to use
Redundant click handler code
It would be better to observe clicks on the document and look for the class names on the target element. Those class names could be put into a mapping of class names to values to be passed to
CSS
The rulesets for
Can be condensed to:
And
Can be condensed to:
1https://www.sitepoint.com/community/t/getelementbyid-vs-queryselector/280663/2
The loop that generates people - an expensive bridge to cross
”...DOM access is actually pretty costly - I think of it like if I have a bridge - like two pieces of land with a toll bridge, and the JavaScript engine is on one side, and the DOM is on the other, and every time I want to access the DOM from the JavaScript engine, I have to pay that toll”
- John Hrvatin, Microsoft, MIX09, in this talk Building High Performance Web Applications and Sites at 29:38, also cited in the O'Reilly Javascript book by Nicholas C Zakas Pg 36, as well as mentioned in this post
Bearing in mind that was stated more than 10 years ago and browsers likely have come along way since then, it is still wise to be aware of this.
This code block makes quite a few redundant DOM look-ups - 30 (based on the value I see assigned to
manNumber) to be exact:for (let i = 0; i < manNumber; i++) {if (i < (manNumber * leftStreet)) {
street[i] = document.getElementById('path-2');
} else {
street[i] = document.getElementById('path-1');
}It would be more efficient to look up those two elements outside the loop - preferably once the DOM is ready...
const path1 = document.getElementById('path-1');
const path2 = document.getElementById('path-2');The inside that loop, use those references:
for (let i = 0; i < manNumber; i++) {
street[i] = i < (manNumber * leftStreet) ? path2 : path1;
}Selecting elements to get innerHTML
The function
fillElement() uses querySelector to get elements by the id attribute:function fillElement (ele) {// Inner
modalInner.innerHTML = document.querySelector('#js-modal-template-'+ele).innerHTML;
}It isn't wrong to use
querySelector to get elements by id but using getElementById() "is definitely faster" 1 (see this jsPerf test for comparison).Redundant click handler code
$('.mic-g').on('click', function (e) {fillElement('cineteca');
modal.replay();
modalOpenTimeline.replay();
});// Belgrado / Kinoteka$('.belgrado-g').on('click', function(e) {
fillElement('kinoteka');
modal.replay();
modalOpenTimeline.replay();
});
//And on and on for more selectors...It would be better to observe clicks on the document and look for the class names on the target element. Those class names could be put into a mapping of class names to values to be passed to
fillElement(). Something like below (untested):const classModalMapping = {
'mic-g': 'cineteca',
'belgrado-g': 'kinoteka',
//...etc.
}
$(document).click(function(e) {
$(e).target.classList.forEach(function(className) {
if (className in classModalMapping) {
fillElement(classModalMapping[className]);
modal.replay();
modalOpenTimeline.replay();
}
});
});CSS
The rulesets for
.modal-text-title and .modal-text-inner both have padding rules where the left and right values are identical. Those can be condensed to a single value.padding: 120px 5% 20px 5%;Can be condensed to:
padding: 120px 5% 20px;And
padding: 0 5% 20px 5%;Can be condensed to:
padding: 0 5% 20px;1https://www.sitepoint.com/community/t/getelementbyid-vs-queryselector/280663/2
Code Snippets
for (let i = 0; i < manNumber; i++) {if (i < (manNumber * leftStreet)) {
street[i] = document.getElementById('path-2');
} else {
street[i] = document.getElementById('path-1');
}const path1 = document.getElementById('path-1');
const path2 = document.getElementById('path-2');for (let i = 0; i < manNumber; i++) {
street[i] = i < (manNumber * leftStreet) ? path2 : path1;
}function fillElement (ele) {Context
StackExchange Code Review Q#159800, answer score: 6
Revisions (0)
No revisions yet.