Speed up processing of TimelinePanel updates on new events #277
Conversation
Avoid twiddling state flags, so that we can avoid redundant re-renders.
| // we won't load this event now, because we don't want to push any | ||
| // events off the other end of the timeline. But we need to note | ||
| // that we can now paginate. | ||
| this.setState({canForwardPaginate: true}); |
There was a problem hiding this comment.
Does the act of changing this ever require a re-render? If not, would it be more appropriate to just make it a regular property on this?
There was a problem hiding this comment.
The logic behind putting it in state was mostly for consistency with canBackPaginate. But yes, we could do that. Or we could ignore changes to canForwardPaginate in shouldComponentUpdate.
There was a problem hiding this comment.
OK - hopefully doing either of those would reduce the number of re-renders further? Personally I'd make them regular properties and keep state for things that affect what's drawn to the screen.
There was a problem hiding this comment.
I need to return to this to fix element-hq/element-web#1427 anyway, so I'll go with it as-is for now.
|
Comment aside, this fix is still valid either way so lgtm. |
Avoid twiddling state flags, so that we can avoid redundant re-renders.