Header: disable level switching if lab loading #51858
Conversation
| @@ -71,6 +72,10 @@ class LessonProgress extends Component { | |||
| } | |||
| } | |||
|
|
|||
| if (this.props.isLabLoading !== nextProps.isLabLoading) { | |||
There was a problem hiding this comment.
can you add a comment as to why this check is needed?
There was a problem hiding this comment.
Does the code make sense in the context of the shouldComponentUpdate function? Basically everything here is just checking changed props that should result in doing a re-render...
There was a problem hiding this comment.
I guess I'm wondering if there's ever a case where we would still want to render a change, even if the lab loading state hasn't changed. Maybe not? But worth explaining why.
There was a problem hiding this comment.
fwiw, we will still render a change for other props changing. This just makes sure we also render a change if the loading state changes.
There was a problem hiding this comment.
ah gotcha, makes sense!
To make things simpler for a lab that supports switching levels without a page reload, this is a proposal to disable switching levels using the header bubbles if a level is currently loading.
Without this change, we would have some issues with level switching initiated by pressing the header bubbles, in which we could end up with multiple requests (for level data, saved code, etc.) arriving asynchronously and potentially out of order. The solution would be to wait for the right set of responses to arrive for the last-selected level, and might also involve waiting for all in-flight requests to be completed before settling on a level to show, and would be non-trivial to get right.