Ho list page #36
Ho list page #36
Conversation
…ocalstorage on user sign out
IamHenryOkeke
requested review from
llunaplanet,
ijayhub,
khadijahnabil and
jmahamed
|
Visit the preview URL for this PR (updated for commit 6703049): https://tcl-72-smart-shopping-list--pr36-ho-listpage-n6gtvenh.web.app (expires Sun, 07 Apr 2024 08:11:28 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: f7f2b0c98e4c5bce6be0f9b2cd44669f154caa67 |
There was a problem hiding this comment.
Everything is working as expected @IamHenryOkeke , amazing work! 🚀
I left a non-blocking comment
src/AuthContext.jsx
Outdated
| const [currentUser, setCurrentUser] = useState(null); | ||
| const [pending, setPending] = useState(true); | ||
|
|
||
| useEffect(() => { | ||
| onAuthStateChanged(auth, (user) => { | ||
| setCurrentUser(user); | ||
| setPending(false); | ||
| }); | ||
| }, []); | ||
|
|
There was a problem hiding this comment.
nice use of React Context 💫. We already have this code, for fetching and detecting user changes, in the useAuth custom hook so this is a duplication. I would recommend to move the pending state/logic to the useAuth hook and use the hook here.
There was a problem hiding this comment.
Thank you Reda for pointing that out. I took a closer look at the code and i figured that the using a context wasn't necessary so I removed the context as a whole. The route protection works just fine regardless. Please review
For an example of how to fill this template out, see this Pull Request.
Description
This code change/update updates the List Page UI and provides route protection to relevant pages
Related Issue
closes #32
Acceptance Criteria
Type of Changes
Bug fixUI enhancementUpdates
Before
After
Testing Steps / QA Criteria
Do a git pull and checkout to ho-listPage branch. Run npm start to start development server. Navigate to list page to see UI updates. Also sign out of the app and try to route to the list and mange-list page. You shouldn't be able to access the pages if not signed in