airbnb / infinity

UITableViews for the web (DEPRECATED)
http://airbnb.io/infinity/
Other
2.8k stars 281 forks source link

Page resize totally breaks ListView #5

Open banks opened 12 years ago

banks commented 12 years ago

Hi,

First off thanks for releasing this code. It's great to have a solution that is so well built and tested rather than rolling the same thing.

One of the biggest issues for me right now though is that the list view cannot handle fluid width/page resizes at all.

I open the ticket more to discuss the issue than to demand a fix since I'm well aware that it's a fairly deep-rooted problem that may not have a satisfactory solution.

It may also be that the "API for notifying ListViews/ListItems when elements change size" listed in your Future work section may address part of this issue.

To share though: Lets assume that it is a common case for list item contents to change height when the page width changes - one example is because it is full of text, or other floated or inline-block elements that wrap around with fluid width.

There is a hard problem here: if the content is text or complicated inline-block elements (as in my case), it is essentially impossible to perfectly model the re-flow and therefore calculate height correction needed to ensure the scrolling will be smooth and gap/overlap free as you scroll back up after window resize. Essentially you have to re-render each "hidden" ListItem and re-measure it and then correct the height of the spacer by the difference. This is going to be very hard to do quickly enough when you may have hundreds of items with possibly thousands of DOM elements hidden.

So the discussion point is: what is the best solution we can find to this issue? If the answer is no solution, or only use fixed-width ListViews, then I think it is a very significant limitation that should be listed up front in the documentation for the library.

This is a current problem for my product so I will update this ticket if any useful solutions come up (and/or contribute a patch if appropriate).

reissbaker commented 12 years ago

Yup, it's a hard problem. Naive resizing was something I meant to get in earlier -- it's in now, but it'll still only work for layouts that don't change their heights. It's just calling repartition internally, which assumes all original sizes are correct; if the (vertical) sizes of items have changed, it'll break.

At the moment, repartition will fail on things that change height because:

  1. it relies on the (mostly-undocumented) clone method of ListItems, which uses cached positioning data instead of recalculating, and
  2. the (awkward) cacheCoordsFor function will incorrectly cache coordinates for ListItems being generated inside of a repartition call.

Neither seem particularly intractable: repartitions could just regenerate the coordinates, once the cacheCoordsFor function is fixed. For a quick fix, all that needs to happen is that the pages need to be entirely removed from the screen at the start of a repartition; cacheCoordsFor should just work after that.

Switching over to a self-balancing tree, instead of a sorted array, and only caching sizing and not positioning (positioning can be inferred from sizing and ordering) will simplify this in the long run. But this seems important enough to handle that it's worth doing earlier than the internal data structure change.

Re: performance: I'm hoping that it shouldn't be too terrible even for a few thousand DOM elements. There's no way around it, though: they have to be in the layout in order to calculate their sizing. But a debounced resize event should go a long way. For reference: the Infinity demos each throw about 12,000 fairly complicated container elements, each with many child elements, into the DOM at startup, so although it's possible there might be a slight hitch after a resize, in even fairly large cases browsers should be able to handle it.

banks commented 12 years ago

Thanks for the response. Having dug through the code here a little more deeply I can appreciate your comments a little more.

I'm encouraged by the fact that cacheCoordsFor already does a bunch of the things that would be needed by resize without being noticeable. My initial thinking on the problem was that having JS mess around adding, measuring and removing from DOM would be too slow in general but currently the code already does just that for every append and is not at all noticeable in my tests. That gives me more hope that having a total re-flow of the page could be performant enough not to be a total usability nightmare on resize.

I also realised that resizing a very long page full of fluid content is a massively disruptive experience in general even if all DOM is present at all times (assuming you are not at or near the top of the page when you do it) so the same attention to super-smooth and non-noticeable updates to page are not such a big deal provided the resize event is de-bounced as you say.

I will endeavour to report back as and when we have time to revisit this. There are currently other more demanding priorities for the project but this will need revisiting eventually.

sawyerh commented 12 years ago

+1 for this to get added — currently unable to use this on a lot of projects because they're fluid.

banks commented 11 years ago

Sorry I've been silent for a while on this. Wanted to fill you in with where I got to.

FWIW I did implement resize handling with a really crude hack of infinty.js as a proof of concept and the suggested technique of measuring all the pages one by one actually works OK.

In the end I could not use infinity.js for my project because:

I hope these pointers are in some way useful. I can probably expand on any point, answer any questions you have and probably provide examples from the code I came up with if any of that is helpful when thinking about future architecture of infinity.js. I did not implement and submit a pull request though as I feel my implementation has probably veered too far away from the general solution here and is too focused on our specific needs to be useful to all.

Let me know if I can help out with further ideas/examples of anything I've mentioned here.

Thanks again for an inspiring project and I certainly learnt a lot and took away a bunch of inspiration from infinity.js.

urbien commented 11 years ago

we are considering switching from masonry.js to infinity.js and need to understand if all the functionality we have now will continue to work. Specifically, when orientation on iphone changes to landscape, masonry is automatically switching from one column to two. Will infinity.js be able to do the same?

banks commented 11 years ago

I've not kept up with changes to infinity since my comment above so I could be wrong but given the lack of any progress on this ticket I'd guess nothing has changed with this regard.

So to sum up the issue here: infinity was not useae for us due to its assumption that you list content will never change size after it is initially added to the page. That means reflow after resize/orientation change is out. I sort of got that working just for window resize with a quick hack but as explained above it still was a long way from stable for us given our requirement to have content which changes height dynamically after being added.

So you may be able to achieve what you want with some reasonable changes but unless it changed a lot and this ticket just got forgotten, you won't be able to support orientation change reflow out of the box.

Comments from anyone with more recent experience welcome.

urbien commented 11 years ago

@banks - thanks for clarification, have you found any other library that removes content from dom for smooth scrolling? I have only located linkedin project, but it was not open sourced, see http://engineering.linkedin.com/linkedin-ipad-5-techniques-smooth-infinite-scrolling-html5. @reissbaker - has the original intent of airbnb changed? Is infinity.js going to evolve, or you guys are busy with other things and are waiting for a community champion to take over the project?

banks commented 11 years ago

@urbien no I've not looked for more libs for this. I came to the conclusion that we had a pretty complex set of requirements that were unlikely to be met by any generic library, and similarly which would be of little value/hard to generalise to suit other people's specific needs. I went into more details above but to add even more, we also ended up with some complex requirements for dynamically loading and manipulating adverts within the stream such that they still look like they are inline with the content but are not being removed and reinserted at high rate as you scroll which dramatically affects click-through rate and hence revenue...

Sorry I can't be of more help. If you take on building something or extending infinity and have questions please ping me I'll be happy to help based on my experience 6 months ago.