Fix: changedMonitors.forEach (montage.js) #4039
Conversation
Iterating through the array was incorrect
|
It does not fix it. In my testing here, I went over every monitor instead of just the changed ones. That also didn't help, because the actual width of the monitor was different, so setting the fixed ratio didn't help. |
| @@ -798,7 +798,7 @@ function initPage() { | |||
|
|
|||
| setInterval(() => { //Updating GridStack resizeToContent, Scale & Ratio | |||
| if (changedMonitors.length > 0) { | |||
| changedMonitors.forEach(function(item, index, object) { | |||
| changedMonitors.slice().reverse().forEach(function(item, index, object) { | |||
There was a problem hiding this comment.
It is not clear to the casual observer why the slice and reverse are neccessary. perhaps a comment would help.
Alternatively a simple for loop instead of a forEach is actually faster and you can just clear the array at the end. Would be easier to read as well.
There was a problem hiding this comment.
At the time of execution of this code, other data may be filled into the "changedMonitors" array. Therefore, it cannot be cleaned after processing all elements. Step by step cleaning is required. This is why "for" is not used.
slice().reverse() - to iterate over a copy of the array from the end, otherwise it will not be possible to correctly remove the processed monitor from the array
There was a problem hiding this comment.
Aha! Excellent content for a comment.
So the slice makes a copy... which we could use for on. Maybe we should just use a while with popping off elements until the array is empty... so that if another item gets added we will process it.
There was a problem hiding this comment.
I'm not ready to say anything yet.
I can't figure out the reason and I can't reproduce the problem because... All my monitors are the same width. The width of the block is completely controlled by GridStack. Is one or more monitors the wrong width? Is there any fundamental difference between monitors with the correct width and those with the wrong one (Monitor type, something else...)? |
|
Id say leave it for now. It's not a big deal. |
|
I made changes
:) |
|
That's got it. Well done. |
Iterating through the array was incorrect
Possible fixes #4037