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
5 changes: 2 additions & 3 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,7 +13,6 @@ function Layout() {
const getUserQueues = useGetUserQueues();

useEffect(() => {
// TODO: Server still returns queues for anonymous users
// Dispatch action only if auth is loaded
if (isLoading === false) {
dispatch(getUserQueues());
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
21 changes: 17 additions & 4 deletions simplq/src/components/common/Nav/LeftNav.jsx
Original file line number Diff line number Diff line change
@@ -1,28 +1,41 @@
/* eslint-disable jsx-a11y/anchor-is-valid */

import React from 'react';
import { smoothScrollTo } from 'utils/scrollingOperations';
import { smoothScrollTo, smoothScrollToHomePageTop } 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();
// get the target div by ID
const element = document.getElementById('target_how_it_works');
let element = document.getElementById('target_how_it_works');
if (element) {
// 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
setTimeout(() => {
element = document.getElementById('target_how_it_works');
smoothScrollTo(element);
}, 0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

lets create a util function onLoadbyId:

/** 
 * 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.
function onLoadById(id, callback) {
  var checkAndExecute = setInterval(function() {
     const element = document.getElementById(id);
     if (element) {
        callback(element);
        clearInterval(checkAndExecute);
     }
  }, 100);
}

then just use it here: onLoadById('target_how_it_works', e =>smoothScrollTo(e))

Copy link
Collaborator

Choose a reason for hiding this comment

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

The above was inspired by this, you can check if there are better ways to do this, though.

}
};
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
4 changes: 2 additions & 2 deletions simplq/src/utils/scrollingOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ 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('/');
}
};