-
Notifications
You must be signed in to change notification settings - Fork 0
feat(authentication): Added login, routes and navigation #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
d340621 to
0790b17
Compare
| <Button | ||
| name="logout" | ||
| variant="tertiary" | ||
| className={styles.dropdownOption} |
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.
disable button when fetching. Change variable name to logoutPending
| const toggleAccordion = (index: number) => { | ||
| setOpenIndexes((prev) => (prev.includes(index) | ||
| ? prev.filter((i) => i !== index) | ||
| : [...prev, index])); | ||
| }; |
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.
use useCallback
| type="button" | ||
| className={styles.navHeaderContainer} | ||
| childrenContainerClassName={styles.navHeader} | ||
| onClick={() => toggleAccordion(index)} |
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.
| onClick={() => toggleAccordion(index)} | |
| onClick={toggleAccordion} |
| )} | ||
| > | ||
| <Button | ||
| name={item.title} |
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.
| name={item.title} | |
| name={index} |
|
|
||
| } | ||
| function Navigation({ navigationItem }: { navigationItem: NavigationItem[] }) { | ||
| const [openIndexes, setOpenIndexes] = useState( |
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.
instead of open indexes can we use the name of the button? or the link?
| import UserContext from '#contexts/UserContext'; | ||
| import { useLoginMutation } from '#generated/types/graphql'; | ||
|
|
||
| import banner from '../../resources/image/redCrossBanner.png'; |
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.
add new absolute route for resources
|
|
||
| const error = getErrorObject(formError); | ||
|
|
||
| const [{ fetching }, triggerLogin] = useLoginMutation(); |
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.
| const [{ fetching }, triggerLogin] = useLoginMutation(); | |
| const [{ loginPending }, triggerLogin] = useLoginMutation(); |
| }); | ||
| navigate('/'); | ||
| } else { | ||
| setError({ email: 'Failed to login!' }); |
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.
Also show and alert
| <Image src={banner} /> | ||
| <form | ||
| className={styles.loginForm} | ||
| onSubmit={(e) => { e.preventDefault(); handleFormSubmit(); }} |
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.
add a separate handler instead of having inline functions. Avoid inlines as much as possible.
| .leftPane { | ||
| display: flex; | ||
| flex-direction: column; | ||
| box-shadow: 0px 4px 4px 0px rgba(227, 227, 227, 0.25); |
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.
avoid this if possible. If not, then its fine.
Changes
This PR doesn't introduce any
console.logmeant for debuggingThis PR includes