Skip to content

Fx login UI#29

Open
the1Riddle wants to merge 3 commits into
bkoimett:mainfrom
the1Riddle:fx_login_ui
Open

Fx login UI#29
the1Riddle wants to merge 3 commits into
bkoimett:mainfrom
the1Riddle:fx_login_ui

Conversation

@the1Riddle

Copy link
Copy Markdown

What I changed

Updated routing so root now serves login:

  • / → LoginPage
  • /login → redirects to /

Also added signup entry points without creating a separate signup page file:

/signup route (reuses LoginPage, since there is no signup page), which is now available on the login page

The login page is updated such that authenticated users are sent to /dashboard.

fixes: #16

Note: that i have not created a signup page/I've just provided a linking to it.

if the page does not exist, it can be created later or integrated in the login page so that it switches between the two
@vercel

vercel Bot commented May 27, 2026

Copy link
Copy Markdown

@the1Riddle is attempting to deploy a commit to the bkoimett's projects Team on Vercel.

A member of the Team first needs to authorize it.

@the1Riddle

Copy link
Copy Markdown
Author

Hello @bkoimett ,

Would you take a moment to review this?

thanks

@llamapreview llamapreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AI Code Review by LlamaPReview

🎯 TL;DR & Recommendation

Recommendation: Approve with suggestions

This PR re-routes the root to the login page and adds a placeholder signup route, but introduces a minor routing inefficiency and a potential UX nuance on the signup page.

💡 Suggestions (P2)

  • client/src/App.jsx: The redirect from /login to / introduces an unnecessary double redirect when unauthenticated users are sent from ProtectedRoute to /login, adding minor complexity and performance overhead.
  • client/src/pages/LoginPage.jsx: Authenticated users clicking the 'Sign up' link are immediately redirected to /dashboard without seeing the placeholder message, creating a potentially confusing experience.

💡 Have feedback? We'd love to hear it in our GitHub Discussions.
✨ This review was generated by LlamaPReview Advanced, which is free for all open-source projects. Learn more.

Comment thread client/src/App.jsx Outdated
Comment thread client/src/pages/LoginPage.jsx Outdated

@bkoimett bkoimett left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

fix the issues in teh bot review

Comment thread client/src/pages/LoginPage.jsx Outdated
@the1Riddle the1Riddle requested a review from bkoimett May 27, 2026 16:44
@the1Riddle

the1Riddle commented May 27, 2026

Copy link
Copy Markdown
Author

kept the initial routing,
/ ---> dashboard
/login ---> login
then kept the added route:
/signup ---> signup

@vercel

vercel Bot commented May 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
theserenityplace Ready Ready Preview, Comment May 28, 2026 5:40pm

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.

Fix: add sigup url on the login page for redirection

2 participants