roeierez / infinite-list

Infinite list in javascript that scrolls in 60fps
623 stars 45 forks source link

promise based height getter? #3

Closed hyusetiawan closed 8 years ago

hyusetiawan commented 9 years ago

Is it possible with the current architecture to support a promise based height getter? I can see cases where images or remotely loaded content that can change heights after the content is loaded.

roeierez commented 9 years ago

I am working on support for 'unknown heights', which basically means the user will not be required to implement the 'itemHeightGetter'. I still need to know the height of the element rendered and planning to do that by query the DOM element after it is rendered. There will be still a reasonable assumption that the size of the element will not change after it is rendered. Does this change answer your use case?

hyusetiawan commented 9 years ago

an example of a use case is, loading an image, by first showing a loading spinner and then the actual image. The loading spinner is the first thing it renders and then the height will change after the final resource in this case image is loaded, based on "size of the element will not change after it is rendered comment it doesn't seem that the unknown heights support this capability?

On Mon, Jun 15, 2015 at 1:51 PM, Roei Erez notifications@github.com wrote:

I am working on support for 'unknown heights', which basically means the user will not be required to implement the 'itemHeightGetter'. I still need to know the height of the element rendered and planning to do that by query the DOM element after it is rendered. There will be still a reasonable assumption that the size of the element will not change after it is rendered. Does this change answer your use case?

— Reply to this email directly or view it on GitHub https://github.com/roeierez/infinite-list/issues/3#issuecomment-112207637 .

roeierez commented 9 years ago

Got you now. Actually this case is supported now. The list will initially render your item with the height retrieved from the 'itemHeightGettet'. Aftet the image is loaded simply invoke the list 'refresh' method and make sure the 'itemHeightGetter' returns the new height for thst item index. Another option is to render the spinner container with the exact size of the image so when the image is loaded the height will not change and you will get a more smooth behavior. The second option is possible only if you know the rendering size of the image.

hyusetiawan commented 9 years ago

ah gotcha, okay, that works, is refresh() light enough to be called frequently?

roeierez commented 9 years ago

Though it will re-render only the items you see on screen, it still a bit overkill for just resize an item. It seems reasonable to add 'itemHeightChangedAtIndex(index)' API method that will only shift the next items and fix the scrolling position (if necessary), what do you think?

hyusetiawan commented 9 years ago

that would work, then it would be up to the developer to update after the size is updated. But, i think it would be better if we can manually set the height ourselves? setItemHeight(index, height)

roeierez commented 9 years ago

I am working on it. I intend to deliver support for 'changing heights' and 'unknown heights' where the developer does not need to pass the height at all. The second option of course might come with a price of a slightly performance decrease since the list will have to query the DOM on every list item rendering. I think I will have it implemented in a week from now.

roeierez commented 9 years ago

It takes more time than I thought but it really looks good. At the simple use case the user will not have to take care about heights at all, the list will detect heights automatically after rendering the DOM element. At the advanced use case where the height is changing after rendering, the user will have two options:

  1. only to notify the list of height changed at an element and the list will detect the new height automatically.
  2. notify the list and give the new height explicitly.

You can observe my work at the variable_heights branch. I added a real example for these cases of a list showing pictures from flickr dynamically with unknown heights of the pictures, but I still need some time to polish, clean the code, make sure the performance is not decreased and that I did not break anything.

hyusetiawan commented 9 years ago

Checked it out, I got an error:

Uncaught TypeError: Cannot set property 'onImageLoaded' of undefined InfiniteList.itemRenderer @ flickr_example.js:104 renderListItem @ flickr_example.js:881 render @ flickr_example.js:814 render @ flickr_example.js:293 (anonymous function) @ flickr_example.js:215 animationStep @ flickr_example.js:766

to reproduce, just click anywhere, or scroll. Tested it on google chrome.

On Thu, Jul 2, 2015 at 9:45 PM, Roei Erez notifications@github.com wrote:

It takes more time than I thought but it really looks good. At the simple use case the user will not have to take care about heights at all, the list will detect heights automatically after rendering the DOM element. At the advanced use case where the height is changing after rendering, the user will have two options:

  1. only to notify the list of height changed at an element and the list will detect the new height automatically.
  2. notify the list and give the new height explicitly.

You can observe my work at the variable_heights branch. I added a real example for these cases of a list showing pictures from flickr dynamically with unknown heights of the pictures, but I still need some time to polish, clean the code, make sure the performance is not decreased and that I did not break anything.

— Reply to this email directly or view it on GitHub https://github.com/roeierez/infinite-list/issues/3#issuecomment-118230727 .

roeierez commented 9 years ago

Sorry for that, I have fixed the issues and the flickr example now works. Thank you.

hyusetiawan commented 9 years ago

Tested it, works really well!! :)

On Fri, Jul 3, 2015 at 10:48 PM, Roei Erez notifications@github.com wrote:

Sorry for that, I have fixed the issues and the flickr example now works. Thank you.

— Reply to this email directly or view it on GitHub https://github.com/roeierez/infinite-list/issues/3#issuecomment-118463581 .

roeierez commented 8 years ago

it is now supported and merged to master. Thanks for your help and sorry for the delay.