fix(BC-173): profile ui updates properly after logging out#20
fix(BC-173): profile ui updates properly after logging out#20mellobacon wants to merge 2 commits intomainfrom
Conversation
|
Do we really want to navigate back to the home page rather than staying on the profile page of the user and just not showing the edit button? |
There was a problem hiding this comment.
Pull request overview
Updates the user menu logout behavior so signing out also navigates the user back to the homepage, ensuring the UI reflects the logged-out state immediately.
Changes:
- Make the “SIGN OUT” dropdown item render as a router link to
/while triggeringauthClient.signOut().
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <DropdownMenuItem | ||
| className="font-mono text-xs font-bold tracking-widest uppercase text-destructive" | ||
| onClick={() => authClient.signOut()} | ||
| render={<Link to="/" />} | ||
| > |
There was a problem hiding this comment.
authClient.signOut() is likely async (it’s invoked with void elsewhere). With render={<Link to="/" />} the navigation may occur before sign-out completes, and any rejection from signOut() is currently unhandled. Consider handling the promise (e.g., void/await) and triggering navigation only after sign-out succeeds (using router navigation in the click handler rather than rendering as a Link), optionally showing an error if sign-out fails.
There was a problem hiding this comment.
@mellobacon Ignore the comment about async here, fire-and-forget is the typical pattern for async handlers in click events and because the AppHeader is always mounted, the navigation to / won't stop the signOut from continuing
There was a problem hiding this comment.
That being said, a better way to do this would be to use better-auth's fetchOptions so that the redirect is within the auth lifecycle
There was a problem hiding this comment.
There was a problem hiding this comment.
In our example here, we'd just use the useNavigate hook from tanstack router to await the nav to / in the success of fetchOptions
@Duxez this is a pretty typical pattern, but another option is to instead invalidate the current router to reload the current route which would take them out of edit but keep them on the profile |
@josh-complex I know it's typical, I also hate it lol |
Haha bet, then @mellobacon lets just call tanstack's |
|
Oh sorry actually, the recommended way with modern tanstack router is by using the relative paths, so stick with my earlier comment about using useNavigate https://tanstack.com/router/latest/docs/guide/navigation#special-relative-paths--and- |
@josh-complex So do we still want it to navigate to the homepage? Or just refresh the current page? |
Well, josh said do it my way, but honestly that'll just become a hassle to fix once admin panels come in so just navigate home in the way josh showed tbh. It's logical for admin views to be navigated away from after logout. I just don't like it when it also happens on public pages lol |
navigates back to the homepage Signed-off-by: mello <mellodev@outlook.com>
page refreshes on logout Signed-off-by: mello <mellodev@outlook.com>
oliverbooth
force-pushed
the
mellobacon/bc-173
branch
from
04c5714 to
d3df59a
Compare
signs out and navigates back to the homepage