Update: Change VirtualScroller.renderItems strategy #900
Conversation
|
Verified that @ConradJChan has signed the CLA. Thanks for the pull request! |
| * @param {number} start - start index | ||
| * @param {number} end - end index | ||
| * @param {number} [end] - end index |
There was a problem hiding this comment.
Should [end] be in brackets?
There was a problem hiding this comment.
I wanted to because if not provided (which is how I call it https://github.com/box/box-content-preview/pull/900/files#diff-ae1d3917492ce0b7a65c4b31408eb5deR282) it will just delete to the end of the listEl's children
src/lib/VirtualScroller.js
Outdated
| @@ -254,7 +254,7 @@ class VirtualScroller { | |||
| } | |||
|
|
|||
| // Create a new list element to be swapped out for the existing one | |||
| const newListEl = this.createListElement(); | |||
| const newListEl = document.createDocumentFragment(); | |||
There was a problem hiding this comment.
The naming here seems a little confusing, since later we append this to this.listEl.
| newListEl.appendChild(newEl); | ||
| } | ||
| const listItems = Array.prototype.slice.call(listEl.children, start, end); | ||
| listItems.forEach((listItem) => listEl.removeChild(listItem)); |
There was a problem hiding this comment.
Do we incur a performance penalty here, since this.listEl is currently mounted? Each removeChild call will force a reflow, right?
There was a problem hiding this comment.
I don't think so because all the li are absolutely positioned
src/lib/VirtualScroller.js
Outdated
| @@ -254,7 +254,7 @@ class VirtualScroller { | |||
| } | |||
|
|
|||
| // Create a new list element to be swapped out for the existing one | |||
There was a problem hiding this comment.
Is this comment still relevant?
Changed the strategy from re-creating the
olon each render, to manipulating the individualli(creating new ones and deleting the old ones outside the "window" into the list).This is because Safari wasn't smart about the cloned elements in the previous strategy and would cause a flicker of the thumbnails on each
renderItemscall