-
Notifications
You must be signed in to change notification settings - Fork 1.6k
chore: Change redirect notification from toast to modal #30747
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This PR replaces the toast-based redirect notification with a modal dialog in the authentication redirect flow.
- Updated
/frontend/src/scenes/authentication/redirectToLoggedInInstance.ts
to import and useLemonModal
for notifications. - The modal includes explicit "Continue" and "Cancel" actions, with the primary triggering a redirect.
- The modal’s onClose callback unconditionally redirects, which may conflict with cancelling actions.
- Uses dynamic import for the modal to handle asynchronous loading.
1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
frontend/src/scenes/authentication/redirectToLoggedInInstance.ts
Outdated
Show resolved
Hide resolved
Working on this now. |
@Twixes feel free to give this a review. I didn't set an automatic count down with a redirect but we could. |
📸 UI snapshots have been updated3 snapshot changes in total. 0 added, 3 modified, 0 deleted:
Triggered by this commit. |
📸 UI snapshots have been updated1 snapshot changes in total. 0 added, 1 modified, 0 deleted:
Triggered by this commit. |
Size Change: -10 B (0%) Total Size: 13.4 MB ℹ️ View Unchanged
|
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Problem
Discussion: https://posthog.slack.com/archives/C01FHN8DNN6/p1743590167887249
The redirect toast is being missed. We should change it to a modal instead. I think this does that, but has the usual 'Joe is a shit coder who is just trying his best, just be glad this isn't in zapfino' caveats on it.
Changes
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Does this work well for both Cloud and self-hosted?
How did you test this code?