Feature/admin login 2#6
Conversation
| authListener(signUserIn) | ||
| }, []) | ||
|
|
||
| function signUserIn(user) { | ||
| setUserLoggedIn(user) | ||
| if (user) { | ||
| history.replace('/') | ||
| } |
There was a problem hiding this comment.
Looks like some issues with auto indent? Or maybe it is Github's problem?
I see like 8 spaces for one indent level 🤔
| {userLoggedIn ? ( | ||
| <DashboardPage /> | ||
| ): ( | ||
| <Redirect to="/login" /> | ||
| )} |
There was a problem hiding this comment.
Let's discuss this one over the meeting - dynamic and declarative routing (especially redirects) like the ones in React as super great in day to day use, but they have some scalability issues (i.e. long term there are some down sides using this approach), so I want to address them specifically.
There was a problem hiding this comment.
- Discuss routing in React application
| import RoundedAvatar from '../components/RoundedAvatar' | ||
| import LoadingAnimation from '../components/LoadingAnimation' | ||
| import firebase from 'firebase' | ||
| import firebase from '../firebase' |
There was a problem hiding this comment.
Right now one of the third-party dependencies is leaking into your app's codebase. In such a case, it would be difficult to remove the dependency or replace it (more often scenario). Let's discuss it in the meeting
There was a problem hiding this comment.
- Discuss abstraction over dependencies
| <PasswordAvatar user={adminUser} onBack={() => setAdminUser(null)}/> | ||
| ) : ( | ||
| <div className='users-container'> | ||
| {users && users.map((user) => |
There was a problem hiding this comment.
Shouldn't users be an empty array in the case there are no users?
2 participants
Use Firebase authentication for admin login and normal user.
For a normal user, the password has been hard-coded from the front-end and saved to Firebase Auth. May have potential security issues.
Add a pointer for cards which are yet to be flipped.