Recreate tracing controller on reconnect. #5071
Conversation
polina-c
changed the title
| tracingPaneController.dispose(); | ||
| tracingPaneController = TracingPaneController(); |
There was a problem hiding this comment.
why does this controller need to be recreated and the other two (profilePaneController and diffPaneController) do not?
There was a problem hiding this comment.
good question that triggered nice refactoring. Thank you!
| @@ -40,6 +40,34 @@ class OfflineFileException implements Exception { | |||
| String toString() => message; | |||
| } | |||
|
|
|||
| class MemoryChildControllers { | |||
There was a problem hiding this comment.
rename to MemoryFeatureControllers to match naming in other screens.
It would be worth investigating state management between the performance page and the memory page to see if we should use a common approach there. The Performance page and the Memory page have a similar setup where each tab contains a somewhat self contained feature. In the performance screen, we have a concept of "activeFeature" and each feature controller extends PerformanceFeatureController, which is an abstract class with lifecycle methods that each feature controller should implement (some examples init, onBecomingActive, setOfflineData, clearData, etc.). This setup is nice from an organization standpoint but also prevents us from doing business logic work in a controller that we don't need to be doing if a feature is not active. Not pressing to do in this PR, but just food for thought.
There was a problem hiding this comment.
thanks
Noted this for the time I will work to support offline mode for memory screen.
|
|
||
| final tracingPaneController = TracingPaneController(); | ||
| /// Sub-controllers of memory controller. | ||
| late final MemoryChildControllers children; |
There was a problem hiding this comment.
nit: rename to featureControllers
There was a problem hiding this comment.
featureControllers is too long. how about just `controllers' ?
I want this expression to be easy to read and take reasonable space:
controller.controllers.diff
RELEASE_NOTE_EXCEPTION=[already covered in https://github.com//pull/4614]
Follow up for #5062 (comment)