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.

Sign up for GitHub

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

Jump to bottom

Create a snackbar to notify the user of important events #226

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Create a snackbar to notify the user of important events #226

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e3ca82e
Add snackbar component baseline
Jan 15, 2020
692c18a
Add loading bar on bottom of snackbar and change out span for button …
Jan 21, 2020
a10d776
Change snackbar to use reducers and implementing it in adding transac…
Jan 22, 2020
2612d3b
Remove SnackBarSymbol and uncomment authentication tests
Jan 22, 2020
b922aa4
Fixed error snackbar from not closing on click
Jan 28, 2020
6844534
linting update
Jan 28, 2020
de8d8bf
even more linting fixes
Jan 28, 2020
04d35b3
Refactor based on PR review
Feb 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions frontend/src/components/atoms/ProjectionRow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const ProjectionRow = styled.div`
align-self: center;
margin-top: 0.2em;
margin-bottom: 0.2em;
word-break: break-word;
}

p:nth-last-child(-n + 3),
Expand Down
23 changes: 23 additions & 0 deletions frontend/src/components/atoms/SnackBarCloseButton.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import styled from 'styled-components';
import { theme } from '../../styling/theme';
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't import the theme from file, this doesn't allow it to update if the theme changes.


export default styled.button`
background-color: ${theme.palette.primary.main};
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be

Suggested change
background-color: ${theme.palette.primary.main};
background-color: ${props => props.theme.palette.primary.main};

Copy link
Contributor

Choose a reason for hiding this comment

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

See example here

border: none;
padding-right: 0.5em;
float: right;
position: absolute;
right: 10px;
top: 1.2em;
color: ${theme.palette.primary.contrast};
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

font-weight: 600;
text-transform: uppercase;
transition: 0.1s linear;
margin-left: 1em;

&:hover {
color: ${theme.palette.danger.main};
Copy link
Contributor

Choose a reason for hiding this comment

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

Same.

transition: 0.1s linear;
cursor: pointer;
}
`;
68 changes: 68 additions & 0 deletions frontend/src/components/atoms/SnackBarContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import React from 'react';
import styled from 'styled-components';
import { theme } from '../../styling/theme';
Copy link
Contributor

Choose a reason for hiding this comment

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

Here as well. Won't comment anymore on this as I think you get the gist ;)

import SnackBarCloseButton from './SnackBarCloseButton';
import SnackBarLoader from './SnackBarLoader';

interface ISnackBarProps {
className?: string;
clicker?: () => void;
Copy link
Contributor

Choose a reason for hiding this comment

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

Name should be updated, this doesn't make it very clear what it does. If it's a handler, use the appropriate type.

content: string;
good: boolean;
speed?: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Then this could also be of type Speed (ref. next comment);

}

const LoaderSpeed = (speed = '6s') => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't be capitalized if not a component.

You could also move the speed out to a type. Will make the typing a bit clearer, and will be a bit easier to use if you don't know what it does.

type Speed = 'fast' | 'medium' | 'slow';
const loaderSpeed = (speed: Speed = 'medium') => {
  switch (speed) {
    case 'fast':
      return '4s';
    case 'medium':
	  return '6s';
    case 'slow':
      return '8s';
  };
};

Copy link
Contributor

Choose a reason for hiding this comment

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

Or even,

const speedMap: Record<Speed, string> = {
  'fast': '4s',
  // ...
}

const loaderSpeed = (speed: Speed = 'medium') => loaderSpeed[speed];

if (speed === 'fast') {
return '4s';
} else if (speed === 'slow') {
return '8s';
} else {
return '6s';
}
};

const SnackBarContainer: React.FC<ISnackBarProps> = ({
className,
clicker,
content,
good,
speed,
}) => {
return (
<div className={className}>
Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid having an optional className prop everywhere, it's probably better to move this out to a styled.div. I.e. instead of wrapping in div className={className}, wrap it in a SnackBarWrapper or something. This is also a bit cleaner in debugging :)

<p>{content}</p>
<SnackBarCloseButton onClick={clicker}>x</SnackBarCloseButton>
Copy link
Contributor

Choose a reason for hiding this comment

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

Clicker should probably be snackBarCloseHandler or similar. And if it's a callback from an onClick on a button, it shouldn't be () => void, it should be React.MouseEventHandler<HTMLButtonElement>, i.e.

interface IBlablablaProps {
  // ...
  onCloseHandler: React.MouseEventHandler<HTMLButtonElement>;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Or if it is on the div, change HTMLButtonElement to HTMLDivElement etc.

<SnackBarLoader
style={{
animationDuration: LoaderSpeed(speed),
}}
/>
</div>
);
};

export default styled(SnackBarContainer)`
color: ${theme.palette.primary.contrast};
background-color: ${theme.palette.background.default};
padding: 0.5em;
border-radius: 3px;
border: 3px solid;
border-color: ${props =>
props.good ? theme.palette.success.main : theme.palette.danger.main};
Copy link
Contributor

Choose a reason for hiding this comment

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

When updating all other occurrences of the theme -> props.theme thing, remember to update these as well.

position: fixed;
bottom: 20px;
left: 20px;
display: inline-block;
min-width: 200px;
max-width: 70vw;
padding-left: 1em;
-webkit-box-shadow: 10px 10px 16px -7px rgba(0, 0, 0, 0.75);
-moz-box-shadow: 10px 10px 16px -7px rgba(0, 0, 0, 0.75);
box-shadow: 10px 10px 16px -7px rgba(0, 0, 0, 0.75);

& > p {
width: 100%;
padding-right: 4em;
}
`;
27 changes: 27 additions & 0 deletions frontend/src/components/atoms/SnackBarLoader.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import styled from 'styled-components';
import { keyframes } from 'styled-components';
import { theme } from '../../styling/theme';

const shrinkBar = keyframes`
from {
Copy link
Contributor

Choose a reason for hiding this comment

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

indentation += 2

width: 100%;
}
to {
width: 0%;
}
`;

export default styled.div`
height: 5px;
background-color: ${theme.palette.primary.contrast};
padding: 0;
margin: 0;
position: absolute;
bottom: 0px;
left: 0px;
animation-name: ${shrinkBar};
animation-duration: 8s;
animation-iteration-count: 1;
animation-timing-function: linear;
border-radius: 0 3px 3px 0;
`;
115 changes: 89 additions & 26 deletions frontend/src/components/molecules/AddTransaction.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,67 @@
import moment from 'moment';
import React from 'react';
import React, { ReactElement, useReducer } from 'react';
import styled from 'styled-components';
import { useAuthState } from '../../store/contexts/auth';
import { useTransactionDispatch } from '../../store/contexts/transactions';
import { TransactionActions } from '../../store/reducers/transactions';
import Collapsable from '../atoms/Collapsable';
import SnackBarContainer from '../atoms/SnackBarContainer';
import Form from './Form';

const initialState = { snax: <div />, content: '' };

interface IState {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
interface IState {
interface ISnackStore {

snax: ReactElement;
content: string;
clicker?: () => void;
}

interface IAction {
type: 'clear' | 'good' | 'bad';
Copy link
Contributor

Choose a reason for hiding this comment

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

Argh, already wrote a lot of comments, but updating the file removed them all 🙃

This doesn't have to be an object.

type SnAction = 'clear' | 'good' | 'bad';

}

const reducer = (state: IState, action: IAction): IState => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, so there are quite a few things here that need to be fixed. Already wrote some of them that later disappeared, so I'll be a bit less verbose this time.

  1. Reducer should be moved to its own file.
  2. You generally shouldn't store render elements in the store. This is bad practise, and will generally result in a lot of trouble. You have no way to update this element, and it will have to be recreated every time the reducer is updated (I think). Furthermore, how it is mounted in the DOM is not clear, and I have a hunch that it will be updated way more times than it has to. You can check this with React DevTools, but tl;dr, this should be reworked. It should be sufficient to only save the text to display along with the variant ('good' etc.).
  3. The good and bad reducer actions do the exact same thing except set the good prop differently. This means that it should probably be generalized, and you should make an addToast action that takes content and variant.

I created a simple example of one way to solve snackbars. This handles more than one in a queue system, but reverting it to only storing one shouldn't be hard.

Now, how would you implement this in liquidator? Probably one of the easier ways would be to do the same as with auth and transaction -- i.e. create a context at the root that allows you to pass the dispatch down, allowing you to create snax from anywhere.

Link

Copy link
Contributor

Choose a reason for hiding this comment

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

Note related to the example;

If you opt for this method of handling state, remember to clear the timeout that removes the snack if the user clicks it. If not, if a similar toast appears later, it will be removed on the timeout of the first snack.

Right now:

  1. snack created; 'hello'
  2. user clicks; removed
  3. snack created; 'hello'
  4. timeout for first snack hits; removes second snack (3)
  5. timeout for second snack hits; does nothing

switch (action.type) {
case 'clear':
return initialState;
case 'good':
return {
content: state.content,
snax: (
<SnackBarContainer
clicker={state.clicker}
good={true}
content={state.content}
/>
),
};
case 'bad':
return {
content: state.content,
snax: (
<SnackBarContainer
clicker={state.clicker}
good={false}
content={state.content}
/>
),
};
default:
return initialState;
}
};

const AddTransaction: React.FC<{ className?: string }> = props => {
const dispatch = useTransactionDispatch();
const auth = useAuthState();
const [state, snaxDispatch] = useReducer(reducer, {
content: '',
snax: <div />,
});

const onButtonClickHandler = () => {
snaxDispatch({ type: 'clear' });
};

const onSubmit = async ({
recurring,
Expand All @@ -22,39 +74,50 @@ const AddTransaction: React.FC<{ className?: string }> = props => {
interval_type,
interval,
}: any) => {
if (!recurring) {
await TransactionActions.doCreateTransaction(
{
company_id: auth!.selectedCompany!,
date,
description,
money: money * 100,
notes,
type,
},
dispatch
);
} else {
await TransactionActions.doCreateRecurringTransaction(
{
company_id: auth!.selectedCompany!,
end_date,
interval,
interval_type,
start_date: date,
template: {
if (!(description.length > 140) && !(notes.length > 140)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Here it's probably better to use guard statements. So instead of wrapping everything in an if-statement, just check your stuff, do your "YOU DID NOT ENTER THINGS ACCORDING TO HOW I WANT THEM"-snack-dispatching, and then return;. That way, you don't have to indent everything and increase the "legibility"-complexity.

if (!validLength) {
  toast('invalid length');
  return;
}

if (!validFormat) {
  toast('invalid format');
  return;
}

// etc.

state.content = 'Transaction added successfully';
state.clicker = onButtonClickHandler;
Copy link
Contributor

Choose a reason for hiding this comment

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

Never mutate state in react! All updates to state should be done with dispatch or setState.

snaxDispatch({ type: 'good' });
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't know if you want this, as this will trigger before you get a response from the server. If you want to wait for that, move it to after await ...doCreateTransaction(....


if (!recurring) {
await TransactionActions.doCreateTransaction(
{
company_id: auth!.selectedCompany!,
date,
description,
money: money * 100,
notes,
type,
},
},
dispatch
);
dispatch
);
} else {
await TransactionActions.doCreateRecurringTransaction(
{
company_id: auth!.selectedCompany!,
end_date,
interval,
interval_type,
start_date: date,
template: {
description,
money: money * 100,
type,
},
},
dispatch
);
}
} else {
state.clicker = onButtonClickHandler;
state.content = 'Too many characters.';
snaxDispatch({ type: 'bad' });
setTimeout(() => snaxDispatch({ type: 'clear' }), 6000);
}
};

return (
<Collapsable heading={<h1>Add new transaction</h1>}>
<div>{state.snax}</div>
<div className={props.className}>
<Form
schema={[
Expand Down
38 changes: 38 additions & 0 deletions frontend/src/stories/index.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import RecurringTransactionOptions, {
IntervalType,
} from '../components/atoms/RecurringTransactionOptions';
import Select from '../components/atoms/Select';
import SnackBarContainer from '../components/atoms/SnackBarContainer';
import TextArea from '../components/atoms/TextArea';
import TransactionEntry from '../components/atoms/TransactionEntry';
import AddTransaction from '../components/molecules/AddTransaction';
Expand Down Expand Up @@ -299,3 +300,40 @@ storiesOf('Input/Select', module)
</Select>
);
});

storiesOf('SnackBarContainer', module)
.addDecorator(fn => (
<div style={{ margin: '2em', background: theme.palette.primary.main }}>
{fn()}
</div>
))
.add('Long', () => (
<SnackBarContainer
good={false}
content="You are currently signed in as [insert user here]. A really long sentence to test max length capacity. It is red. It will only break after hitting 70vw."
speed="fast"
/>
));

storiesOf('SnackBarContainer', module)
.addDecorator(fn => (
<div style={{ margin: '2em', background: theme.palette.primary.main }}>
{fn()}
</div>
))
.add('Fast', () => (
<SnackBarContainer good={true} content="Short and fast" speed="fast" />
));
storiesOf('SnackBarContainer', module)
.addDecorator(fn => (
<div style={{ margin: '2em', background: theme.palette.primary.main }}>
{fn()}
</div>
))
.add('Medium', () => (
<SnackBarContainer
good={true}
content="Medium speed and medium length."
speed="medium"
/>
));