Wanted to share a few updates on this.
The -50 value seems to be coming from here (in <NoteListWrapper />)
const noteListSize = useMemo(() => {
return {
width: props.size.width,
height: props.size.height - controlHeight,
};
}, [props.size, controlHeight]);
When the list is toggled off, the height gets to zero and the controlHeight, which is 50px from theme.topRowHeight makes it negative. The original value of the height is computed in the layout handlers. The computed height should infact be zero when a component is not visible and thus I did not consider changing them just for this particular use case.
Now, I tried a bunch of different approaches to fix the height issue for this component. Here is what I could observe :
( <NoteListWrapper/> wraps <NoteList /> which in turn renders the final <ItemList /> )
- Checking for negative height in
<ItemList /> doesn't really work. Once the negative height is sent to the component the top, bottom, visibleItemCount and scrollTop indexes all are set to wrong values. When called again, even though the height value is corrected, these values remain wrong and the layout gets messed up in the new render. Their calculation is also dependent on one another so fixing them within the component seems infeasible.
- Currently, toggling the list only updates
<NoteList /> sizes . <ItemList /> properties are not changed and its size is passed directly with no updates in between the parent components.
As of now, to me the solution seems to be adding state related to the size(i.e height,width) in one of the above components, preferably the <NoteListWrapper /> (functional component) and when the list is toggled, restore the value of the height which prevents it from taking incorrect values. (similar to a componentWillReceiveProps, checking if the list is toggled using the visible prop and updating the height to the correct value ).
Something like :
(height is a state variable and previousVisible is the previous value of props.visible)
useEffect(() => {
if (props.visible === previousVisible){
setHeight(props.size.height - controlHeight);
}
else{
//List toggled
setHeight(previousHeight);
}
},[props.size.height,props.size.width,controlHeight]);
Since this is a functional component, the previous props are made available using a custom hook like this. I made an initial implementation and it seems to work for now. I'll discuss the complete implementation here if and when required.
@roman_r_m @laurent I wanted to know your thoughts on this before proceeding or opening a PR. Also , how should I go about testing this thing ?
Thanks!