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

change size of rendered item #56

Closed
Fr1kki3 opened this issue Oct 20, 2017 · 18 comments
Closed

change size of rendered item #56

Fr1kki3 opened this issue Oct 20, 2017 · 18 comments

Comments

@Fr1kki3
Copy link

Fr1kki3 commented Oct 20, 2017

Good day based on the source code I can see that there is an onSizeChange(), but I cannot find anything on how to change the size in such a mater that this event is triggered. Any pointers on what too look for?

@naqvitalha
Copy link
Collaborator

onSizeChanged is raised by the scroll container to inform that dimensions of list have been changed. It causes a relayout to fit items accordingly. This only works if you set canChangeSize to true. In case of web, only works with useWindowScroll since there are no resize events to listen to on <div/>
Also, the title mentions "rendered item", what exactly are you looking for?

@Fr1kki3
Copy link
Author

Fr1kki3 commented Oct 20, 2017

Using react native, I have items in my list and when I click on one I want it to expand in other words I want to change the size of this item in the list and fit the rest of the list below it.

my last attempt was as follows:
renderActivity(type, activityModel, index){
select = () =>{
if(this.height === itemHeight){
//expanded
this.height = deviceHeight * 0.5;
}
else {
//compressed
this.height = itemHeight;
}
}

return(
  <View>
    <ActivityItemView activity={activityModel}
      arrowImage={require('../Assets/images/golden_arrow.png')}
      selectItem={() => {select()}}/>
    <View style={styles.line}/>
  </View>
);

}

this is the method used as the rowRenderer

I realize that changing the height property here or at least in the way I did it does not actually change it, this.height has the same value before and after setting it so my question is where can I change the dimensions of the item in the list so the size actually changes and updates list layout

@naqvitalha
Copy link
Collaborator

What you're looking for is non deterministic rendering. Set forceNonDeterministicRendering to true on recyclerlistview. Post that simply render the bigger item and the list will auto relayout itself. Please not that in non deterministic scenario the height and widths mentioned in layout provider are considered as close estimates. They will not be enforced on items, you will have to do that yourself.

If your heights are deterministic in nature then you can also just return updated value in layout provider. e.g,

(type, dim, index) => {
let dataAtIndex = getData(index)
if(dataAtIndex.isExpanded){
// set heights
}
else {
//set different heights
}
}

forceNonDeterministicRendering is much simpler compared to the latter approach but it is relatively slower, may not be noticeable but it technically causes more layout thrashing.

If you need further help, try sharing a sample on expo. Will make is much more simple.

@Fr1kki3
Copy link
Author

Fr1kki3 commented Oct 20, 2017

Great that helps thank you.

@naqvitalha
Copy link
Collaborator

Great, closing.

@Fr1kki3
Copy link
Author

Fr1kki3 commented Oct 23, 2017

Good day, I am using extremely big lists and therefore speed is very important. So I went with the second approach.

layoutProvider = new LayoutProvider(
index => {
let dataAtIndex = this.state.data.getDataForIndex(index);
if(!dataAtIndex.isExpanded){
return ViewTypes.COLLAPSED;
}
else {
return ViewTypes.EXPANDED;
}

        },
        (type, dim) => {
          if(type == ViewTypes.COLLAPSED){
            dim.width = deviceWidth;
            dim.height = itemHeight;
          }
          else {
            dim.width = deviceWidth;
            dim.height = deviceHeight * 0.5;
          }
        }

);

cloneRows(activityModel){
activityModel.isExpanded = !activityModel.isExpanded;

this.setState({
  data: this.dataProvider.cloneWithRows(
    this.state.filteredActivities.slice(this.state.startIndex, this.state.endIndex)
  )
});

}

renderActivity(type, activityModel, index){
return(

<ActivityItemView activity={activityModel}
arrowImage={require('../Assets/images/golden_arrow.png')}
expand={() => this.cloneRows(activityModel)}/>


);

render(){
console.log(this.state.data);
return(

{

      this.state.filteredActivities ?
        (
          <RecyclerListView
            style={styles.list}
            layoutProvider={this.layoutProvider}
            dataProvider={this.state.data}
            rowRenderer={(type, activityModel, index) => this.renderActivity(type, activityModel, index)}
            onEndReached = {() => this.handleLoadMore()}
            onEndThreshold = {15}
            renderFooter={()=> this.renderFooter()}
            canChangeSize={true}
            />
        )
        : null
    }
  </View>
);

}

expanded
expanded
compressed
collapsed

This works good for expanding and compressing the views however I realized this causes a bug on the list view where if I expand an item, compress it again and scroll down the rest of the list looses its layout and gets part of the previously extended item smashed over it.

broken

What could cause the recyclerListView to do this?

@Fr1kki3
Copy link
Author

Fr1kki3 commented Oct 23, 2017

Disabling recycling fixed the problem, but I would still like to know why this is necessary?

@naqvitalha naqvitalha reopened this Oct 23, 2017
@naqvitalha
Copy link
Collaborator

@Fr1kki3 We've had similar layouts here and haven't faced any such issue. Expand collapse never caused this for us. This is definitely unexpected.
Also, please don't disable recycling, that will greatly increase memory usage and slow down the list.

If you're in a hurry you can go with non deterministic rendering. From looking at your layout it should be pretty fast too. Let's try to find out the issue here, can I take a look at the code if possible? or, can you provide a repro on snack? It would be very helpful to us as well since we have not faced this issue before. The thing is if it's working fine without recycling then that means layout compute is correct. If that's true things should never overlap.

One more thing I could not understand the slice logic in the code, can you explain?

@naqvitalha
Copy link
Collaborator

Also data: this.dataProvider.cloneWithRows() you should ideally do this on the last dataProvider. So call this on the dataProvider inside the state.

@naqvitalha
Copy link
Collaborator

I've created a small sample. You can try running it on expo using the link: https://expo.io/@naqvitalha/snack-rykYFcoaW

If you want to view the code use expo snack link: https://snack.expo.io/rykYFcoaW

Color is not changing on snack. There is some issue on re render logic on their end. The latter is the same code built using their sdk. In any case, I'm not seeing the same issue. Check out.

@Fr1kki3
Copy link
Author

Fr1kki3 commented Oct 23, 2017

slice is for paging, if I scroll fast I get white spaces where rows still needs to be rendered, the chance of this increases If there is many icons or images that needs to load(much less than FlatLists though). Paging solved this problem. Thanks for the advice on the dataProvider. I will continue trying to reproduce the problem outside of my production app and let you know ass soon as I am able. At the moment I am also unable to reproduce this problem on snack/expo.

@naqvitalha
Copy link
Collaborator

Okay, great. Regarding the blanks, I have few questions:

  1. Did you test on dev mode or release/minify?
  2. What device and OS did you use?
  3. Do you have a valid check in rowHasChanged and shouldComponentUpdate inside the component?
  4. Are you rendering everything even in the "COLLAPSED" state?

From looking at the template it shouldn't go blank. We have lot of complex views here as well and we've been able to prevent blanks in almost all cases. Android fares better on quick scroll compared to iOS though.

Also, if you remove paging do you still get the layout issue?

@Fr1kki3
Copy link
Author

Fr1kki3 commented Oct 24, 2017

Only debug mode so far.

Both Android and IOS, in my case it was worse on my android tablet.(My device is slower than most by default)

The data provider has a valid rowHasChanged, As far as shouldComponentUpdate is conserned I saw it in your snack project yesterday and that is the only place I have seen it till now. Also when I took your snack project scaning the QR code from the first link everything worked good while if I scan it from the second link(code) the size changed but the component it self didn't update on toggle (stayed gray and had the collapsed text) until I commented out the shouldComponentUpdate method then everything went smooth again. I will try to add it and see what happens.

Yes on the initial render everything is in collapsed state.

Yes removing the paging makes no difference to the layout problem.

@Fr1kki3
Copy link
Author

Fr1kki3 commented Oct 24, 2017

Ok so regarding the layout bug I had a display component inside the row that should enable if the row is expanded. Using a state inside the component to determine if it should be enabled rather than the dataItem sent as prop solved the problem

@naqvitalha
Copy link
Collaborator

@Fr1kki3 Great that you were able to solve it. So, you enabled recycling post that right? But we still don't know the root cause :(

Coming back to the blank problem:

  1. Debug mode is way slower than release. On Android it's very easy to test on release. Shake device, open dev setting, uncheck dev and check minify, reload! Having list scroll without halts was one of our goals. I really think you should try to get rid of paging. With RecyclerListView it is possible IMO.

  2. Both links that I shared have the exact same code. I, myself, don't know why the snack one has some issue. Never seen such issue anywhere else. I'll report it to expo.

  3. In case of deterministic rendering rowHasChanged is enough. But in non deterministic it is must to have shouldComponentUpdate to maintain perf.

Let me know your results!

@Fr1kki3
Copy link
Author

Fr1kki3 commented Oct 24, 2017

Yes recycling is on, regarding the blanks release mode solved it without pager until i opened my extreme case (13000 items) then app runs out of memory the moment I open the view, witch is rather expected to an extend

@naqvitalha
Copy link
Collaborator

It is expected if data is bulky. 13000 items is a lot of data moreover, you'd be getting it over the bridge which means first there'll be a copy of it on native end. Ideally you'll need to do data virtualization in this case. The reason we don't pass data field in layout provider is for the same reason. To do data virtualization you'll need to fetch data inside component and render it yourself. That way once the cell is recycled you'll also let go of the data. Holding 13000 items in memory might be impossible even in cases where individual items are not very large. Paging will help with initial load but what if someone actually pulls in all data by scrolling to the end?

@naqvitalha
Copy link
Collaborator

Since the issue is resolved. Closing 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

2 participants