Skip to content
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

feat(ui): added logout feature #133

Closed
wants to merge 0 commits into from

Conversation

prajjwalkapoor
Copy link
Member

@prajjwalkapoor prajjwalkapoor commented Jul 9, 2022

closes #128

@Samy-33 Samy-33 requested a review from Aksh-Bansal-dev July 9, 2022 10:04
Copy link
Member

@Aksh-Bansal-dev Aksh-Bansal-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would recommend you to setup ESlint and prettier in your editor.
You can also run npm run lint:fix or npm run format to fix these linting errors.

ui/src/components/Sidebar/Sidebar.tsx Outdated Show resolved Hide resolved
Copy link
Member

@Aksh-Bansal-dev Aksh-Bansal-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Member

@Samy-33 Samy-33 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add tests for this.

import styles from './styles.module.css';

export const Sidebar: React.FC<{}> = () => {
const navigate = useNavigate();
const logoutHandler = () => {
window.sessionStorage.removeItem('token');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logout is a multi step process. Following should be done in order:

  1. When user clicks on logout, a DELETE request should be sent to the API at /auth/token:revoke
  2. While the request is being processed, the loading should be shown and the whole screen should be disabled.
  3. Once the request succeeds, remove the token from sessionStorage and navigate user to /login

const navigate = useNavigate();
const logoutHandler = () => {
window.sessionStorage.removeItem('token');
navigate('/login', {replace: true});
Copy link
Member

@Samy-33 Samy-33 Jul 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can omit replace option.

replace: true option replaces the current entry in url history stack with the one we are navigating to. We want current URL to exist in the history.

// isActive: boolean;
}

export const SidebarRow: React.FC<Props> = props => {
return (
<div className={styles.container_row}>
<div className={styles.container_row} onClick={props.onClick}>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will throw an error when onClick is not provided by the parent.

Something like this should make sure that we don't call onClick when it's null.

<div className={styles.container_row} onClick = {() => props.onClick && props.onClick()}>

@Samy-33
Copy link
Member

Samy-33 commented Jul 9, 2022

@prajjwalkapoor, please comment on #128 so that it can be assigned to you.

@Samy-33
Copy link
Member

Samy-33 commented Jul 14, 2022

@prajjwalkapoor, are you still working on this. It has been idle for some time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[UI] Logout feature
3 participants