fix: sign-out crash from disposed drawer context

[dawidvdh]

Summary

• The sign-out handler called navigator.pop() to close the drawer
  before showDialog, disposing the drawer's context
• showDialog then used the dead context, causing either a silent failure or crash
• Fix: show the confirmation dialog while the drawer is still mounted, pop both dialog and drawer on confirm

Closes #5652

I am not a flutter developer so this could be totally wrong but I was exploring this cool project and thought, ah lets try fix something minor, excuse and close if this is just noise.

[greptile-apps]

Greptile Summary

This PR fixes a crash on sign-out by reordering the drawer close and dialog show operations. Previously, navigator.pop() disposed the drawer's BuildContext before it was passed to showDialog, causing a silent failure or crash; now the dialog is shown while the drawer is still mounted, and both the dialog and drawer are dismissed inside the confirm callback.

Confidence Score: 5/5

Safe to merge — correctly fixes a real crash with no regressions introduced.

The fix is minimal, targeted, and uses the correct Flutter pattern (show dialog while parent context is still mounted, dismiss both in the confirm callback). The only remaining note is a pre-existing P2 around the context.mounted guard always evaluating to false after the drawer pop, which likely has no practical impact if the app uses a Firebase auth-state listener for post-sign-out navigation.

No files require special attention.

Important Files Changed

Filename	Overview
app/lib/pages/settings/settings_drawer.dart	Fixes sign-out crash by showing the confirmation dialog before popping the drawer, then popping dialog and drawer in sequence on confirm; a minor pre-existing issue with context.mounted always being false after the drawer pop is noted.

Sequence Diagram

sequenceDiagram
    participant User
    participant Drawer
    participant Dialog
    participant AuthService

    Note over Drawer: Before fix (broken)
    User->>Drawer: tap Sign Out
    Drawer->>Drawer: navigator.pop() → context disposed ❌
    Drawer->>Dialog: showDialog(context: disposed_ctx) → crash/silent fail

    Note over Drawer: After fix
    User->>Drawer: tap Sign Out
    Drawer->>Dialog: showDialog(context: valid_ctx) ✅
    User->>Dialog: tap Confirm
    Dialog->>Dialog: Navigator.of(ctx).pop() — close dialog
    Dialog->>Drawer: Navigator.of(context).pop() — close drawer
    Dialog->>AuthService: SharedPreferencesUtil().clear()
    Dialog->>AuthService: AuthService.instance.signOut()
    Dialog->>Drawer: if (context.mounted) routeToPage(...) — likely false

Loading

Comments Outside Diff (1)

1. app/lib/pages/settings/settings_drawer.dart, line 496-501 (link)

   context.mounted guard likely always false after drawer pop

   Navigator.of(context).pop() closes the drawer synchronously, after which context (the drawer's BuildContext) becomes unmounted. By the time await SharedPreferencesUtil().clear() and await AuthService.instance.signOut() complete, context.mounted will be false, so routeToPage(context, const AppShell(), replace: true) never executes.

   This was already broken in the original code (the drawer was popped before showDialog was even called), so sign-out navigation likely relies on a Firebase auth-state listener rather than this call. If that's the case this is just dead code worth noting. If the app does depend on this routeToPage call to navigate after sign-out, you'd need to capture the parent navigator before the drawer opens (e.g. final rootNavigator = Navigator.of(context, rootNavigator: true)) and use it here instead.

_{Reviews (1): Last reviewed commit: "fix: show sign-out dialog before closing..." | Re-trigger Greptile}

[beastoin]

nvm man btw do you have any chance to test it?

@dawidvdh

[mdmohsin7]

Thanks for the PR, was fixed in one of my recent PRs

[github-actions]

Hey @dawidvdh 👋

Thank you so much for taking the time to contribute to Omi! We truly appreciate you putting in the effort to submit this pull request.

After careful review, we've decided not to merge this particular PR. Please don't take this personally — we genuinely try to merge as many contributions as possible, but sometimes we have to make tough calls based on:

• Project standards — Ensuring consistency across the codebase
• User needs — Making sure changes align with what our users need
• Code best practices — Maintaining code quality and maintainability
• Project direction — Keeping aligned with our roadmap and vision

Your contribution is still valuable to us, and we'd love to see you contribute again in the future! If you'd like feedback on how to improve this PR or want to discuss alternative approaches, please don't hesitate to reach out.

Thank you for being part of the Omi community! 💜