-
-
Notifications
You must be signed in to change notification settings - Fork 122
fix: hide sidebar and chat UI on login screen using session_state #77
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
fix: hide sidebar and chat UI on login screen using session_state #77
Conversation
Is this PR in its final version @PragnyaKhandelwal? Did you check the discussion thread I had mentioned? It's a bit unclear to me. There are merge conflicts... Resolve them and provide a deployment link and relevant screenshots. Thanks. |
Closes #71 Rationale for this changeAs discussed in the discussions for contributors working on Authentication, two approaches were suggested: placing the login UI inside the Journaling section or authenticating the entire app. I chose to implement the authentication across the entire app to ensure a more seamless and secure user experience. What changes are included in this PR?
Are these changes tested?Yes, they have been tested locally and on the deployed [Streamlit app] Login Page![]() (Separate login UI with username/password and signup prompt) Post-login View![]() (Login redirects to full app interface. Sidebar and UI are no longer visible without login.) Are there any user-facing changes?Yes:
Notes
Thank you for reviewing this PR! 🙏 I appreciate your time and effort in maintaining this project. |
Hi @eccentriccoder01 👋 As per the discussion thread on authentication, I’ve now implemented a separate login page to ensure that the entire app is protected by authentication (not just the journaling section). The sidebar and chatbot UI are now hidden until login Let me know if there’s anything you'd like me to adjust or improve—happy to collaborate further! 🙌 |
Hey @PragnyaKhandelwal, I checked out your deployment at https://pragnyakhandelwal-talkheal-talkheal-o9x3ll.streamlit.app/. The login feature you set up looks really effective! I noticed a small but important issue: every time the page refreshes, you have to log in again, even when switching from the Yoga tab to the TalkHeal tab, etc. Would you prefer to address this in the current pull request or defer it to a future issue? Additionally, I noticed that this pull request has been closed; could you please reopen it for merging? Thank you! |
Hi @eccentriccoder01 , Thank you very much for reviewing my PR and for your kind words about the login feature! I appreciate the detailed feedback regarding the login persistence issue. Given that addressing the session persistence across page refreshes and tab switches will require a deeper refactor and some additional work, I would prefer to defer that enhancement to a future issue, which I will gladly take ownership of. For now, I believe this current PR delivers valuable improvements that the rest of the contributors can benefit from immediately. It would be great if you could merge this portion and label the PR/work accordingly. This way, development can continue smoothly while I focus on implementing the persistent login solution next. Thanks again for your time and support—I’m looking forward to working on the session persistence enhancement shortly! Best regards, |
Thank you very much for confirming this, @PragnyaKhandelwal. The authentication and sidebar issues you addressed are indeed excellent solutions. I will create another issue concerning the refresh-login problem. Please feel free to address it or any other issues, and I will assign them to you accordingly. I'm bumping this PR up to level 3 because you've put in a ton of effort. Thanks for making TalkHeal better! |
Which issue does this PR close?
Closes #71 — Hide Sidebar and Chat UI on Login Screen
Rationale for this change
To enhance user experience and security, this PR ensures that the sidebar, chat interface, and profile sections are only shown after successful authentication. This aligns with the broader recommendation to extend authentication coverage across the entire app, rather than limiting it to just the Journaling section.
What changes are included in this PR?
st.session_state['is_logged_in']
.st.rerun()
post-login to ensure full UI refresh.components/login_page.py
.pink.png
) intoassets/
.Are these changes tested?
✅ Yes — Tested locally using mock/test users to simulate login behavior and verify UI visibility.
Are there any user-facing changes?
Yes:
Potential Merge Conflict Warning
users.db
because a local test record was deleted and replaced with mock data for development purposes. This doesn't affect core functionality and can be resolved during merge.Additional Notes
This PR follows the suggested approach of extending authentication to the entire app, as noted in the project contribution guidelines for authentication features (level 3).
More refinements can follow after feedback or upon merging this foundational structure.