Restrict Navigation Bar Links Based on User Authentication Status

[bbland1]

Description

This code change improves the user experience to reduce confusion on function usability when signed in and out of the application. It dynamically adapts the navbar to only show when a user is logged in and prevents "crafty" users from manually navigating to protected routes and unestablished routes by adding redirects back to the homepage.

There was some own confusion of being able to navigate to pages when working on other issues and not understanding why I wasn't seeing what I thought there should be only to realize I wasn't logged in. If I was having that issue a new user could land on the application and attempt to navigate and not realize they are missing out because they aren't logged in. We want to provide a good user experience that directs people to the proper use of the application.

Stumbling upon this issue did make me realize that authentication while handled by firebase still has a UI component that we can think about other than the direct login button. Having the UI change as we have different user states allows for a subconscious understanding (that is fostered by the rest of the web) of how our application should work from the moment of signup.

Related Issue

Closes #26

Acceptance Criteria

• The sign in button is on the homepage when no user is logged in
• The sign out button is in the nav bar when a user is logged in and there is no sign in button
• The nav bar is only visible when a user is logged in
• Prevent manual navigation to routes that a user has to be logged into see

Type of Changes

	Type
✓	💯 Enhancement
✓	♻️ Refactor
✓	🐛 Bug Fix

Updates

Before

Screen.Recording.2024-08-26.at.3.37.51.AM.mov

After

Screen.Recording.2024-08-26.at.3.38.12.AM.mov

Testing Steps / QA Criteria

• From your terminal, pull down this branch with git pull origin bb/navbar-auth-status and check that branch out with git checkout bb-/navbar-auth-status
• Then npm ci to install the newly added dependencies locally and npm start to launch the app.
• Open a second terminal to run the typescript compiler using tsc --noEmit --watch to see any type issue.
• If still signed in from previous usage of the app, make sure to sign out and sign back in.
• When signed out you should only see the sign in button on the homepage with the "hello from the homepage message".
• In the URL bar add /list to the end and see that the page will automatically redirect to the homepage if no user is signed in. The same will happen if /manage-list is added.
• After seeing the redirect of non-logged in users to the homepage if manually changing the page click the sign in button.
• When logged in the nav bar will appear back at the bottom of the page and can be used as it previously was.
• Hit sign out and get redirected back to the home page on successful log out.

[github-actions]

Visit the preview URL for this PR (updated for commit 673abb0):

https://tcl-77-smart-shopping-list--pr27-bb-navbar-auth-statu-znrvqtnm.web.app

_{(expires Sun, 08 Sep 2024 06:43:48 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: b77df24030dca7d8b6561cb24957d2273c5e9d72}

[RossaMania]

I like it! It makes sense, and can prevent future headaches later! We're already checking for a user to be logged in in our issue for this week.

[bbland1]

With commit 36e2def I wanted to confirm that the getUser function added to the ProtectedRoute component used for context to pass the user to the routes we are protecting can't be used in a route not protected. I added it to the Home component and the app did crash. With an issue of being unable to destructure the user information, and wouldn't load the app.

Screen.Recording.2024-08-30.at.1.45.21.PM.mov

[tannaurus]

Last piece of "food for thought" here comes from experience in working in very large React apps: eventually the directory structure of directories like "views" can be very confusing. I find a great solution to this is to have a directory like "views" have two sub directories: something like "authenticated" and "unauthenticated". Our List, ManageList, and others could live in the new authenticated directory, and the new PageNotFound and potentially others could live in the unauthenticated directory

When making changes like that, there's two approaches: one is preferred, but sometimes the other is unavoidable. The preferred option here, that would work well for this work, would be to create a new PR that is branched off of this one (or wait until this merges and branch it off of main), and do the file movements in that PR. Something important with this path would be to only move the files, don't change anything else. This allows you to say in your MR "just moving things around" , which makes it infinitely easier for reviewers, as well as your teammates who will likely have to deal with merge conflicts.

The other solution would be to do the changes here, which I would advise you always try to avoid because it can make the diffs seem much larger than they really are and make reviewing the "real" changes harder.

[bbland1]

This is really good info. I was starting to feel like maybe "reorganization" was on the forefront, but with more files in play I didn't think it was something that would typically be attached to a PR where code has been changed. So knowing that there is multiple approaches, and a preferred method, is really nice.

I'm gonna take the create PR off of this one and move because I just also prefer that. It seems cleaner and easier to deal with.

[tannaurus]

Exceptional work @bbland1 ! I love these changes. Can we hold off on merging this until the other two PRs have been merged?

[tannaurus]

Now that this is in place, we probably want to scan the codebase to remove all User | null where its no longer needed, and use the new getUser function where we previously used the old one

[bbland1]

I think with fixing my merge conflicts of the main after the other two PRs got merged I call all the User | null that were suppose to be there any more. I think the only place I see it now is in places where the protectedRoute isn't used like the App.tsx, Home.tsx, Layout.tsx and in protectedRoute.