WebReflection / domdiff

Diffing the DOM without virtual DOM
ISC License
214 stars 14 forks source link

Enhancing list "scrolling" use case #4

Closed cpinnix closed 6 years ago

cpinnix commented 6 years ago

Hey @WebReflection, I looked into this array re-rendering issue a bit more but on the domdiff side. Codepen here https://codepen.io/cpinnix/pen/PRyjve. It looks like making multiple modifications to an array (changing more than one element) at a time causes domdiff to re-render everything. If you comment line 56 in the codepen and open chrome you can see the difference.

Reference ticket in hyperhtml here https://github.com/WebReflection/hyperHTML/issues/219.

WebReflection commented 6 years ago

Thanks for the follow up.

causes domdiff to re-render everything

Let's start saying that this is not true at all. The fact you see in console node highlighted does not mean these are re-rendered. The highlight happens when you move a node too.

TL;DR nothing here is re-rendered

If you comment line 56 in the codepen and open chrome you can see the difference.

The snabdom also seems to be not optimized for this user case.

Basically, from 1,2,3,4,5 the next button would perform, one after the other, the following operation (note: this is not about re-rendering, this is just about moving nodes)

This happens in here: https://github.com/WebReflection/domdiff/blob/master/esm/index.js#L65

The node is found live and moved before the other one.

Possible enhancement

Let's state this is not a bug, and it's also not such huge performance issue, but I agree there's room for improvements in here.

One way to enhance this specific use case is to find out if the range index-tilltheend matches the new array anyhow and, if that's the case, place all new nodes at once where the current element is placed so that there are 3 moves instead of 7.

However, for all the benchmarks I could do this hasn't really be a major concern while the also is super reliable as it is now so ... I'll keep this open but I don't have any plan to prioritize it.

Meanwhile, you can go on because nothing is re-rendered so if that was your concern, fear not, it's just the DOM moving a bit more than it should have.

cpinnix commented 6 years ago

Thanks for the explanation! I have updated the codepen to include some animation: https://codepen.io/cpinnix/pen/PRyjve. The animation is subtle, but you can see the numbers animate colors over 5 seconds after clicking previous, when, on the other hand, they change instantly after clicking next. Basically, the animation behaves differently depending on the direction one moves. We are looking for a way to make these animations run the same forward and backward when we change more than one item in the array.

I'm still trying to understand exactly what is going on in domdiff, but looking at the code after https://github.com/WebReflection/domdiff/blob/master/esm/index.js#L65 it seems like the order of items in the array matters when there are multiple changes.

WebReflection commented 6 years ago

can you describe that demo with words?

I've no idea what I am looking at, and I don't see any animation.

I don't understand expectations and the code is not super clear neither to me.

Thanks.

cpinnix commented 6 years ago

Hey hey! Sure.

I enhanced the demo again. Codepen here https://codepen.io/cpinnix/pen/PRyjve.

Basically, this is a carousel. There is a list of items and we are rendering a subset of those items into view (five in this case). When the user clicks next, the subset moves one item to the right; the first item to the left is removed from view and a new item is added to the right. When this happens, all other items should slide smoothly to the left. When the user clicks previous, the reverse should happen; the last item to the right is removed from view and a new item is added to the left.

The smooth sliding animation only occurs when clicking the previous button. But when the next button is clicked, the smooth sliding animation doesn't happen.

With that said, if I uncomment line 66 in the codepen, it works as expected, there is smooth sliding left and right.

WebReflection commented 6 years ago

Hey @cpinnix I think I've fixed your demo: https://codepen.io/WebReflection/pen/EEOEVO

It was worth investigating a bit 'cause as result I have improved all inner segments and not just your use case.

Thanks for being patience, providing demos and answering my questions.

Best Regards

WebReflection commented 6 years ago

P.S. use version 0.4.0 or, in Code Pen, just use https://unpkg.com/domdiff to bring in the latest

cpinnix commented 6 years ago

@WebReflection Amazing! This looks great. Thanks so much for your help and great work on this library. It is very cool. Talk soon.

Best, Charlie