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

Moved token and getUserQueues selectors to prevent useless side effects #556

Merged
merged 9 commits into from
Feb 28, 2021
12 changes: 6 additions & 6 deletions simplq/src/components/Layout/Layout.jsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import React, { useEffect } from 'react';
import { useGetUserQueues } from 'store/asyncActions';
import { useDispatch } from 'react-redux';
import { useAuth0 } from '@auth0/auth0-react';
import Footer from 'components/common/Footer';
import Loading from 'components/common/Loading/Loading';
import { useGetUserQueues } from 'store/asyncActions';
import { useDispatch } from 'react-redux';
import Routes from './Routes';
import styles from './Layout.module.scss';

Expand All @@ -13,8 +13,8 @@ function Layout() {
const getUserQueues = useGetUserQueues();

useEffect(() => {
// TODO: Server still returns queues for anonymous users
// Dispatch action only if auth is loaded
// All the backend API calls that should happen at the start goes here.
// They will the dispached as soon as Auth0 has initilised.
if (isLoading === false) {
dispatch(getUserQueues());
}
Expand All @@ -23,8 +23,8 @@ function Layout() {
return (
<div className={styles['box']}>
<div className={styles['content']}>
{/* TODO: Since we have better way for managing loading status
remove conditional loading of the layout once it is handled elswhere */}
{/* We load the main app content only after Auth0 has been initilised.
This helps ensure that no backend API calls are made before auth the initilisation */}
<Loading isLoading={isLoading}>
<Routes />
</Loading>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ import styles from './ClickableLogo.module.scss';
export default (props) => {
const history = useHistory();
const defaultOnClick = () => {
history.push('/');
smoothScrollToHomePageTop();
smoothScrollToHomePageTop(history);
};
return (
<div className={styles['logo']} onClick={props.onClick ? props.onClick : defaultOnClick}>
Expand Down
16 changes: 13 additions & 3 deletions simplq/src/components/common/Nav/LeftNav.jsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
/* eslint-disable jsx-a11y/anchor-is-valid */

import React from 'react';
import { smoothScrollTo } from 'utils/scrollingOperations';
import { smoothScrollTo, smoothScrollToHomePageTop, onLoadById } from 'utils/scrollingOperations';
import { useHistory } from 'react-router';
import styles from './Nav.module.scss';
import LoginButton from '../LoginButton';

const LeftNav = ({ open, toggleClose }) => {
const history = useHistory();

const scrollToHowItWorks = () => {
// Close the navbar on click
toggleClose();
Expand All @@ -15,14 +18,21 @@ const LeftNav = ({ open, toggleClose }) => {
// element is on the current page, just have to scroll to it
smoothScrollTo(element);
} else {
window.location.href = '/#target_how_it_works';
history.push('/');
// wait till page loads before getting element
onLoadById('target_how_it_works', smoothScrollTo);
}
};
return (
<div>
<ul className={styles['left-nav']} open={open}>
<li>
<a tabIndex={0} href="/">
<a
role="link"
tabIndex={0}
onKeyDown={() => smoothScrollToHomePageTop(history)}
onClick={() => smoothScrollToHomePageTop(history)}
>
Home
</a>
</li>
Expand Down
3 changes: 1 addition & 2 deletions simplq/src/components/common/Nav/Navbar.jsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import React from 'react';
import { smoothScrollToHomePageTop } from 'utils/scrollingOperations';
import styles from './Nav.module.scss';
import Burger from './Burger';
import Logo from '../ClickableLogo';

export const Navbar = () => {
return (
<nav className={styles['navbar']}>
<Logo onClick={smoothScrollToHomePageTop} />
<Logo />
<Burger />
</nav>
);
Expand Down
6 changes: 2 additions & 4 deletions simplq/src/components/pages/Admin/AdminPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import React, { useState, useEffect, useCallback } from 'react';
import { useAuth0 } from '@auth0/auth0-react';
import { useDispatch, useSelector } from 'react-redux';
import { selectTokens, selectQueueName } from 'store/selectedQueue';
import { selectQueueName } from 'store/selectedQueue';
import Ribbon from 'components/common/Ribbon';
import Tour from 'components/common/Tour';
import { useGetSelectedQueue } from 'store/asyncActions';
Expand All @@ -19,14 +19,12 @@ let timeoutId;
export default (props) => {
const queueId = props.match.params.queueId;
const queueName = useSelector(selectQueueName);
const tokens = useSelector(selectTokens);
const description = queueName && 'Ready to share';
const dispatch = useDispatch();
const getSelectedQueue = useGetSelectedQueue();

const [toursteps, setToursteps] = useState(getToursteps(window.innerHeight));
const { isAuthenticated } = useAuth0();

const update = useCallback(() => {
clearTimeout(timeoutId);
dispatch(getSelectedQueue({ queueId }));
Expand Down Expand Up @@ -58,7 +56,7 @@ export default (props) => {
/>
)}
<div className={styles['main-body']}>
<TokenList tokens={tokens} queueId={queueId} />
<TokenList queueId={queueId} />
<SidePanel queueId={queueId} />
</div>
</div>
Expand Down
9 changes: 6 additions & 3 deletions simplq/src/components/pages/Admin/TokenList.jsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import React from 'react';
import Loading from 'components/common/Loading/Loading';
import { useSelector } from 'react-redux';
import { selectTokens } from 'store/selectedQueue';
import Token from './Token';
import styles from './admin.module.scss';

function TokenList({ tokens }) {
const Dispaly = () =>
function TokenList() {
const tokens = useSelector(selectTokens);
const ListContent = () =>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Using this selector in admin page was resulting in an unnecessary admin page code re-render, so moved here since only TokenList should be affected by token slice changes.

tokens.length === 0 ? (
<p>Your queue has been created and is currently empty. Waiting for people to join...</p>
) : (
Expand All @@ -14,7 +17,7 @@ function TokenList({ tokens }) {
return (
<Loading isLoading={tokens === undefined}>
<div className={styles['token-list']}>
<Dispaly />
<ListContent />
</div>
</Loading>
);
Expand Down
1 change: 1 addition & 0 deletions simplq/src/components/pages/Home/LandingPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export default () => {
if (isAuthenticated) {
subtitle = `Hi ${user.name}, welcome back!`;
}

return (
<div id="target_top" className={styles['landing-page']}>
<div data-aos="zoom-in">
Expand Down
9 changes: 8 additions & 1 deletion simplq/src/components/pages/Home/MyQueues.jsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import React from 'react';
import React, { useEffect } from 'react';
import DeleteIcon from '@material-ui/icons/Delete';
import IconButton from '@material-ui/core/IconButton';
import { useHistory } from 'react-router';
import { useDeleteQueue } from 'store/asyncActions';
import { selectQueues } from 'store/queues';
import { useDispatch, useSelector } from 'react-redux';
import { setInfoPopupMessage } from 'store/appSlice';
import styles from './Home.module.scss';

export default () => {
Expand All @@ -13,6 +14,12 @@ export default () => {
const queues = useSelector(selectQueues);
const deleteQueue = useDeleteQueue();

useEffect(() => {
if (queues) {
dispatch(setInfoPopupMessage(`Number of queues fetched: ${queues.length}`));
}
}, [dispatch, queues]);

const handleDelete = (e, queue) => {
// Don't trigger parent's onClick
e.stopPropagation();
Expand Down
11 changes: 1 addition & 10 deletions simplq/src/store/appSlice.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint-disable no-param-reassign */
import { createSlice } from '@reduxjs/toolkit';
import { getUserQueues, deleteQueue, getQueueStatusByName, joinQueue } from 'store/asyncActions';
import { deleteQueue, getQueueStatusByName, joinQueue } from 'store/asyncActions';

const appSlice = createSlice({
name: 'appSlice',
Expand All @@ -21,15 +21,6 @@ const appSlice = createSlice({
},
},
extraReducers: {
[getUserQueues.pending]: (state) => {
state.infoText = `Loading queues...`;
},
[getUserQueues.rejected]: (state, action) => {
state.errorText = action.error.message;
},
[getUserQueues.fulfilled]: (state, action) => {
state.infoText = `Number of queues fetched: ${action.payload.queues.length}`;
},
[deleteQueue.pending]: (state) => {
state.infoText = `Deleting queue...`;
},
Expand Down
21 changes: 19 additions & 2 deletions simplq/src/utils/scrollingOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,28 @@ export const smoothScrollTo = (targetElement) => {
});
};

export const smoothScrollToHomePageTop = () => {
export const smoothScrollToHomePageTop = (history) => {
const element = document.getElementById('target_top');
if (element) {
smoothScrollTo(element);
} else {
window.location.href = '/';
history.push('/');
}
};

/**
* Execute a callback as soon as an element is available on the DOM.
*
* id - element id to wait on
* callback - callback to execute as soon as the element becomes available. The
* element is passed to the callback and it is triggered.
* */
export const onLoadById = (id, callback) => {
const checkAndExecute = setInterval(() => {
const element = document.getElementById(id);
if (element) {
callback(element);
clearInterval(checkAndExecute);
}
}, 100);
};