-
Notifications
You must be signed in to change notification settings - Fork 128
Feature/SignoutFrontend #438
base: develop
Are you sure you want to change the base?
Changes from all commits
26ba7b5
0ce50d9
b99d5e0
b4c53ce
4ea3e83
411cb66
cf5a22d
31f4737
34b45c0
7e97e76
0f8ff3c
c443db5
1c4c1f9
d922394
290d8f2
0e9d04d
97b2571
bfca17e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| // scss variables | ||
| $border-color: #000000; | ||
| $text-color-2: #000000; | ||
| $text-color: #ffffff; | ||
| $hover-color: #49a82e; | ||
| $background-color: #ffffff; | ||
|
|
||
| .userGreet { | ||
| color: $text-color; | ||
| margin: 23px; | ||
| } | ||
|
|
||
| .userGreet a { | ||
| padding: 0px; | ||
| } | ||
|
|
||
| .userProfilePic { | ||
| width: 32px; | ||
| height: 32px; | ||
| border-radius: 50%; | ||
| display: inline; | ||
| vertical-align: middle; | ||
| } | ||
| .dropdownHeader { | ||
| margin: 10px; | ||
| cursor: pointer; | ||
| } | ||
|
|
||
| .userGreetMsg { | ||
| display: inline; | ||
| vertical-align: middle; | ||
| margin-right: 10px; | ||
| color: $text-color; | ||
| } | ||
|
|
||
| .userGreetMsg:hover { | ||
| color: $hover-color; | ||
| } | ||
|
|
||
| .myProfileWrapper { | ||
| padding-bottom: 15px; | ||
| text-align: center; | ||
| border-bottom: 1px solid $border-color; | ||
| } | ||
|
|
||
| .myProfile:hover { | ||
| color: $hover-color; | ||
| } | ||
|
|
||
| .myProfile { | ||
| color: $text-color-2; | ||
| text-decoration: none; | ||
| } | ||
|
|
||
| .signOutWrapper { | ||
| padding-top: 15px; | ||
| text-align: center; | ||
| border-top: 1px solid $border-color; | ||
| } | ||
|
|
||
| .signOutWrapper > button { | ||
| all: unset; | ||
| cursor: pointer; | ||
| } | ||
|
|
||
| .signOutWrapper button:focus-visible { | ||
| outline: $border-color 5px auto; | ||
| } | ||
|
|
||
| .signOut:hover { | ||
| color: $hover-color; | ||
| cursor: pointer; | ||
| } | ||
|
|
||
| .dropdownContent { | ||
| display: flex; | ||
| flex-direction: column; | ||
| align-items: center; | ||
| color: $text-color-2; | ||
| border: 2px solid $border-color; | ||
| height: 100%; | ||
| border-radius: 8px; | ||
| width: 100%; | ||
| justify-items: center; | ||
| } | ||
|
|
||
| .dropDownContentWrapper { | ||
| width: 100%; | ||
| height: 100%; | ||
| display: flex; | ||
| flex-direction: column; | ||
| justify-content: center; | ||
| } | ||
|
|
||
| .dropDownContentWrapper div { | ||
| width: 100%; | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| } | ||
|
|
||
| .dropDown { | ||
| background-color: $background-color; | ||
| width: 180px; | ||
| position: absolute; | ||
| overflow: hidden; | ||
| right: 35px; | ||
| border-radius: 8px; | ||
| box-shadow: 0 10px 25px #0000001a; | ||
| } | ||
|
|
||
| .dropDownOpen { | ||
| height: 115px; | ||
| transition: height 0.25s ease; | ||
| z-index: 50; | ||
| } | ||
|
|
||
| .dropDownClose { | ||
| pointer-events: none; | ||
| transition: height 0.25s ease; | ||
| height: 0; | ||
| z-index: 0; | ||
| } | ||
|
|
||
| @media screen and (max-width: 970px) { | ||
| .userGreet { | ||
| margin: 0px; | ||
| } | ||
|
|
||
| .dropDown { | ||
| width: 130px; | ||
| right: 15px; | ||
| top: 45px; | ||
| } | ||
| } | ||
|
|
||
| @media screen and (max-width: 500px) { | ||
| .dropDown { | ||
| width: 110px; | ||
| right: 15px; | ||
| top: 45px; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| import React, { useEffect, useState, useRef } from 'react'; | ||
| import Link from 'next/link'; | ||
| import styles from './dropdown.module.scss'; | ||
|
|
||
| const Dropdown = ({ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. where are the PropTypes defined for this component?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess because This is JavaScript file not a TypeScript file,I don't know that we have to define PropTypes here also, please guide me. |
||
| isLoggedIn, | ||
| USER_PROFILE_URL, | ||
| SIGN_OUT_URL, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should be coming directly from the constants file, why are we passing it as a prop?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because I tried to make the component re-usable. |
||
| userData, | ||
| DEFAULT_AVATAR, | ||
|
Comment on lines
+6
to
+10
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why are some props upper case and some camelCase?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. because both props that are used in camelCase are useState hooks. let me know If I can change it or not. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think so |
||
| }) => { | ||
| const [isOpen, setIsOpen] = useState(false); | ||
| const toggleDropdown = () => setIsOpen((prevState) => !prevState); | ||
| const modalClose = useRef(); | ||
|
|
||
| const signOut = () => { | ||
| fetch(SIGN_OUT_URL, { | ||
| method: 'GET', | ||
| credentials: 'include', | ||
| }).then(() => { | ||
| window.location.reload(); | ||
| }); | ||
| }; | ||
|
Comment on lines
+16
to
+23
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if it fails? where are we handling the failure?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yess got it I will look into it. thank you. |
||
|
|
||
| useEffect(() => { | ||
| const handler = (event) => { | ||
| if (!modalClose.current.contains(event.target)) { | ||
| setIsOpen(false); | ||
| } | ||
| }; | ||
| document.addEventListener('mousedown', handler); | ||
| return () => { | ||
| document.removeEventListener('mousedown', handler); | ||
| }; | ||
| }); | ||
|
|
||
| return ( | ||
| <div className={styles.userGreet} ref={modalClose}> | ||
| <div | ||
| role="button" | ||
| tabIndex={0} | ||
| className={styles.dropdownHeader} | ||
| onClick={toggleDropdown} | ||
| onKeyDown={toggleDropdown} | ||
vinayak-trivedi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| > | ||
| <div className={styles.userGreetMsg}> | ||
| {isLoggedIn ? `Hello, ${userData.firstName}!` : `Hello, User!`} | ||
| </div> | ||
| <img | ||
| className={styles.userProfilePic} | ||
| src={isLoggedIn ? `${userData.profilePicture}` : `${DEFAULT_AVATAR}`} | ||
| alt="Profile Pic" | ||
| /> | ||
| </div> | ||
|
|
||
| <div | ||
| className={` ${styles.dropDown} ${ | ||
| isOpen ? styles.dropDownOpen : styles.dropDownClose | ||
| }`} | ||
| > | ||
| <div className={styles.dropdownContent}> | ||
| <div className={styles.dropDownContentWrapper}> | ||
| <div className={styles.myProfileWrapper}> | ||
| <Link href={USER_PROFILE_URL}> | ||
| <a className={styles.myProfile}>My profile</a> | ||
| </Link> | ||
| </div> | ||
| <div className={styles.signOutWrapper}> | ||
| <button | ||
| type="submit" | ||
| className={styles.signOut} | ||
| onClick={signOut} | ||
| > | ||
| Sign Out | ||
| </button> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| export default Dropdown; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cant, we look to create this as a generic re-usable dropdown component? so it can be used across members? Right now it's only used for Signing users out.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also not sure why is this named a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have created a re-usable dropdown component please let me know if I made any wrong choice. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,8 +7,10 @@ import { | |
| USER_DATA_URL, | ||
| USER_PROFILE_URL, | ||
| NAVMENU, | ||
| SIGN_OUT_URL, | ||
| } from '@constants/AppConstants'; | ||
| import Link from 'next/link'; | ||
| import Dropdown from '../dropdown'; | ||
|
|
||
| import styles from './navbar.module.scss'; | ||
|
|
||
|
|
@@ -112,26 +114,13 @@ const Navbar = () => { | |
| </Link> | ||
| )} | ||
| {isLoggedIn && ( | ||
| <div className={styles.userGreet}> | ||
| <Link href={USER_PROFILE_URL}> | ||
| <a> | ||
| <div className={styles.userGreetMsg}> | ||
| {`Hello ${ | ||
| isLoggedIn ? `${userData.firstName}` : 'User' | ||
| }!`} | ||
| </div> | ||
| <img | ||
| className={styles.userProfilePic} | ||
| src={ | ||
| isLoggedIn | ||
| ? `${userData.profilePicture}` | ||
| : `${DEFAULT_AVATAR}` | ||
| } | ||
| alt="Profile pic" | ||
| /> | ||
| </a> | ||
| </Link> | ||
| </div> | ||
| <Dropdown | ||
| isLoggedIn={isLoggedIn} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When will the value be Also, there is no logical sense in importing these constant values in Navbar component and passing them in Dropdown as props, you should rather import them in Dropdown component itself. |
||
| USER_PROFILE_URL={USER_PROFILE_URL} | ||
| userData={userData} | ||
| DEFAULT_AVATAR={DEFAULT_AVATAR} | ||
| SIGN_OUT_URL={SIGN_OUT_URL} | ||
| /> | ||
| )} | ||
| </div> | ||
| )} | ||
|
|
@@ -187,26 +176,13 @@ const Navbar = () => { | |
| </Link> | ||
| )} | ||
| {isLoggedIn && ( | ||
| <div className={styles.userGreet}> | ||
| <Link href={USER_PROFILE_URL}> | ||
| <a> | ||
| <div className={styles.userGreetMsg}> | ||
| {isLoggedIn | ||
| ? `Hello, ${userData.firstName}!` | ||
| : `Hello, User!`} | ||
| </div> | ||
| <img | ||
| className={styles.userProfilePic} | ||
| src={ | ||
| isLoggedIn | ||
| ? `${userData.profilePicture}` | ||
| : `${DEFAULT_AVATAR}` | ||
| } | ||
| alt="Profile Pic" | ||
| /> | ||
| </a> | ||
| </Link> | ||
| </div> | ||
| <Dropdown | ||
| isLoggedIn={isLoggedIn} | ||
| USER_PROFILE_URL={USER_PROFILE_URL} | ||
| userData={userData} | ||
| DEFAULT_AVATAR={DEFAULT_AVATAR} | ||
| SIGN_OUT_URL={SIGN_OUT_URL} | ||
| /> | ||
vinayak-trivedi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| )} | ||
| </li> | ||
| )} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are we not using
rem, emhere?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beacuse I never worked with them and thought px will do the work, can you please let me know why you are suggesting me to use
rem,emor I can use%There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://dev.to/theodorusclarence/back-to-basic-should-we-use-rem-em-or-pixel-1hd0