Skip to content

Fix a bug with the review history on disconnect experience - #8985

Merged
kenzieschmoll merged 5 commits into
flutter:masterfrom
kenzieschmoll:broken-offline
Mar 4, 2025
Merged

kenzieschmoll merged 5 commits into
flutter:masterfrom
kenzieschmoll:broken-offline

Conversation

@kenzieschmoll

Copy link
Copy Markdown
Member

This was broken by the new screenControllers lifecycle logic added in #8908 and #8916. After the lifecycle changes, the review history on disconnect logic was getting disposed before it got a change to execute. Now we handle this logic in app.dart where the rest of the screen controller lifecycle is managed.

@kenzieschmoll
kenzieschmoll requested a review from a team as a code owner March 4, 2025 17:27
@kenzieschmoll
kenzieschmoll requested review from bkonyi and removed request for a team March 4, 2025 17:27
// When disconnecting from an app, prepare the offline state for reviewing
// history.
screenControllers.forEachInitialized((screenController) {
if (screenController is OfflineScreenControllerMixin &&

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small suggestion: Add a getter to ScreenController (e.g. hasOfflineSupport) so that we we don't have to do OfflineScreenControllerMixin type check

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack. It is beneficial to do the is check so that screenController is type promoted to OfflineScreenControllerMixin, which allows us to call the methods provided by that mixin. If we used a getter hasOfflineSupport instead for example, we'd have to cast screenController to OfflineScreenControllerMixin before calling maybePrepareDataForReviewingHistory

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah okay! LGTM then

@kenzieschmoll
kenzieschmoll merged commit 52d26f4 into flutter:master Mar 4, 2025
@kenzieschmoll
kenzieschmoll deleted the broken-offline branch March 4, 2025 20:41
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

Successfully merging this pull request may close these issues.

2 participants