Addition of login button which shows Hello, <username> , if user is logged in
#64
Conversation
js/login.js
Outdated
| @@ -0,0 +1,16 @@ | |||
| const userLogin = document.querySelector("#login-btn") | |||
|
|
|||
| const setUserName = (name) => { | |||
There was a problem hiding this comment.
Arrow functions are not supported in IE and Opera Mini.
Preferable to use ES5 syntax here.
js/login.js
Outdated
| @@ -0,0 +1,16 @@ | |||
| const userLogin = document.querySelector("#login-btn") | |||
There was a problem hiding this comment.
Can we have this declaration inside the function itself? It's not being used anywhere else.
js/login.js
Outdated
| "credentials": "include" | ||
| }) | ||
| .then(res => res.json()) | ||
| .then(res => setUserName(res.first_name)) |
There was a problem hiding this comment.
Can the function name be a bit more descriptive? Like setNameIfFound?
There was a problem hiding this comment.
Done with the changes as per review. Thanks!
js/login.js
Outdated
|
|
||
| let url = "https://staging-api.realdevsquad.com/users/self" | ||
| let url = "https://staging-api.realdevsquad.com/users/self"; | ||
|
|
||
| fetch(url, { |
There was a problem hiding this comment.
What if we could call the fetch after the DOM Content is loaded?
We can directly shift this code inside the eventListener of DOMContentLoaded but it wouldn't be much readable. For that we could wrap the fetch call in a function and pass that function to the eventListener.
Why am I saying so is that if someone moves the <script> tag in the <head> , since the JS has referred the some DOM content, it won't find it and fail.
There was a problem hiding this comment.
Done with the changes. Thanks for the review!
Rucha1499
changed the title
Hello, <username> , if user is logged in
Updates README.md to include fix for CORS error
Added the login button on welcome site. It displays " Hello username! ", if the user is logged in. The "username" is the first name of user. Fixes #60
