Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

is it possible to have variable element height? #4

Open
trungpham opened this issue Jan 14, 2015 · 23 comments
Open

is it possible to have variable element height? #4

trungpham opened this issue Jan 14, 2015 · 23 comments

Comments

@trungpham
Copy link

I'm looking at a few different scroll list component. None of them seems to support variable element height.

@nmn
Copy link
Owner

nmn commented Jan 14, 2015

react-infinity doesn't currently have variable height support. That said I'm building version 2.

I'm trying to decide whether to add support to react-infinity or just build a separate component.
The basic challenge comes down to calculating the height dynamically from the DOM in a performant way.

Do you think you would want a component where you have to pass the element height?

@mablack
Copy link

mablack commented Jan 21, 2015

That would fit the use case that I have; I need to render a list of maybe 3 different components, and I know the height of each type of component in advance.

@nmn
Copy link
Owner

nmn commented Feb 6, 2015

I should be able to handle that in v2. It will take a couple of weeks to be done.

@pandaiolo
Copy link

+1 for pinterest-like layout !

Also, my use case is the same : I can provide the element height. However, for use cases when the height is unknown, there is already masonry. There is a react component, react-masonry-mixin, not sure how it can play with react-infinity.

@nmn
Copy link
Owner

nmn commented Feb 9, 2015

Thanks for the feedback.

Here are my thoughts on the topic, if I can get some consensus, I can add pinterest like layout soon.

  • The main problem with layout + animation + offscreen culling is knowing the size of the elements
  • In react-infinity, I ask for a global size attribute to solve the problem and make calculations simple
  • Instead (or with an option) I can get the size of elements from the data being used to render the elements. With this if there are variable sized elements, I can easily do a pinterest-like layout
  • It would still be impossible to do flexible layouts (where the width of elements changes with available width) as that basically makes layout totally unpredictable
  • As for a masonry like system for rendering elements where size is not known, I could use a temporary hidden buffer to render them just to get their size before rendering them for real. This would be pretty complicated to implement. Another solution, that I have used for smaller set of elements is to render with a default size for all elements, and then letting the elements call a callback with their actual size and constantly re-adjust the layout position with that. Even with this, culling off-screen elements can be quite a problem. Also with the large number of DOM operations to getSize, there would be a big performance hit.

As I mentioned, I'm working on v2 or react-infinity, with better API, documentation and an example playground. I'm thinking of now adding just the simple case for pinterest-like layouts where you can pass in the size of elements with the data.

But the new code-base is based on smaller components, and all non-essential part of the code like handling scrolling and window-resizes has been factored out into a wrapper component. I can see the buffer system working as another wrapper component.

Let me know your feedback on these things.

@natew
Copy link

natew commented Feb 22, 2015

What about a solution where you give a "range" of heights. Another solution would be having the elements tell their height in advance. In all honesty that solves 99% of use cases.

First one is say you have a list with titles, some titles are 1 line, some up to 4. You could specify (minHeight: 50, maxHeight: 100) and react-infinity would just assume the worst-case scenario and keep that many elements in the DOM.

The other case is for example the twitter feed with images. Some elements are fixed at one size, while others are fixed at a much bigger size. In that case why not have your data objects keep the size inside them. Each element in the array has .size. We know all picture ones are scaled down, while all text ones are fixed at another size.

I know some people would want truly dynamic, but like I said I think this solves just about all options and avoid the #1 no-no: don't touch the DOM.

By the way I'm working on integrating this with reapp.io list views am impressed with usage on the Scribbler. I'm running into some troubles though, has it been tested with a recent version of React (I'm using 0.13 beta)? I can open a ticket but I've run into two issues so far, the second which seems a bit hard to debug.

@natew
Copy link

natew commented Feb 22, 2015

Also want to say if you develop v2 in the open I'd be happy to jump in. Infinite lists are a big feature for what I'm building and I'd love to chip in.

@nmn
Copy link
Owner

nmn commented Feb 22, 2015

@natew I'm primarily looking at the solution where the size would passed with the data. This is the solution you are talking about as well.

I guess, I'll hold off on the more dynamic solutions for now.

As for developing in the open, I'll push my work on a new branch so you and others can chip in. I should have done this a long time ago, thanks for the reminder.

The main concern is I would need to explain the new architecture I have in mind, but I can write a quick readme to explain that.

@nmn
Copy link
Owner

nmn commented Feb 22, 2015

Another interesting thing to note:

React-Canvas has a helper to get the size of text. You give it some text, font-size and font-family and available width, and it tells you the resulting height. This done using an offscreen canvas, so it's much faster than the DOM.

Keeping that in mind, I would like to add a method, where your child component can have a static method that can accept props and return the size.

@nmn
Copy link
Owner

nmn commented Feb 22, 2015

Also, as for the issues you've faced, I have done ZERO testing on 0.13.
I tend to wait for releases to become stable to test on them.

But it seems to work fine on 0.12.2 for me.

(BTW the implementation on the Scribbler is getting old too, I need to update that)

@nmn
Copy link
Owner

nmn commented Feb 22, 2015

quick update, I think I know the issue with 0.13, I was using react/lib/mergeInto which has been removed in 0.13

That is at least one of the issues.

@natew
Copy link

natew commented Feb 24, 2015

Going to check this out again today. This demo would really be killer with reapp. Excited for 2.0.

@natew
Copy link

natew commented Feb 24, 2015

By the way after a quick glance, any way you can make the animations an optional piece? We have a requestAnimationFrame animation mixin already built in that I'd use to integrate. This isn't super important, I could submit a pull request once 2.0 comes out.

@nmn
Copy link
Owner

nmn commented Feb 24, 2015

@natew animations are supposed to be optional. I always factor out my libraries from real projects. As of now, all the work to put a good API is still pending.

@BobWalsh
Copy link

BobWalsh commented Jun 4, 2015

Just wondering how this project is progressing - react infinity looks great, but since I'm rendering really long documents composed of 1000's of

tags, need variable height.

@nmn
Copy link
Owner

nmn commented Jun 4, 2015

I have really been super busy, but I have some free time coming up, will finish an initial 2.0 release by sunday.

@nmn
Copy link
Owner

nmn commented Jul 6, 2015

As you have probably noticed, I have not had the time to update the project much. 2.0 has had some development, but it has not happened on Github. I don't want to give time commitments again as I keep missing them, but let me assure you that 2.0 with variable height support is still coming.

@pandaiolo
Copy link

Update of my use case : full width, vh height units, so it's fixed but it's not pixels :) So yep, I'd be glad to see more flexibility on items height. 👍

@dilizarov
Copy link

@nmn I see above you mention React-Canvas. It looks like you suggest it for the sake of it having some method. Looks like if one needs variable heights they can use React-Canvas over React-Inifnity, right? Or is that being baked into React-infinity as well.

@nmn
Copy link
Owner

nmn commented Nov 8, 2015

@dilizarov I only mentioned React-Canvas as it has a function for measuring text rendering. I see a way to use that in non-canvas contexts.

@dilizarov
Copy link

@nmn Ah yes, I misread and now I understand.

I've been thinking... While it is one thing to have dynamic heights, you might be able to simplify the problem by simply having a minimalHeight property.

For example, in feeds, I can easily tell you what the minimal height would be for a given element. Knowing this minimalHeight, you can determine the maximum number of elements that could fit into the container. Following this, you'll know that container/minimalHeight = number of elements to keep on DOM. Sure, you may have a few additional elements here and there outside of the viewport, but realistically, that in itself is a huge performance gain.

When dealing with a feed with varying heights, and having 10,000 items potentially in the list... having 10 versus 30 items on the DOM because of minimal height really isn't substantial and still boosts performance tremendously.

@wub
Copy link

wub commented Jul 11, 2016

Any progress on this (in this or another repo)?

@nmn
Copy link
Owner

nmn commented Jul 11, 2016

it's taking a while. But I am working on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants