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

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
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
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
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import React from 'react';
import styled from 'styled-components';
import SnackBarCloseButton from './SnackBarCloseButton';
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 SnackBarLoader from './SnackBarLoader';

interface ISnackBarProps {
content: string;
good: boolean;
className?: string;
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);

clicker?: () => void;
}

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> = ({
content,
good,
className,
speed,
clicker,
}) => {
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
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
Original file line number Diff line number Diff line change
@@ -1,15 +1,67 @@
import moment from 'moment';
import React from 'react';
import React, { useReducer, ReactElement } 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 Form from './Form';
import SnackBarContainer from '../atoms/SnackBarContainer';

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

Choose a reason for hiding this comment

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

Storing a react element in a store like this is generally not recommended, as it has to recreate this component every time the reducer updates. I must admit I've never seen this done before, so +1 for creativity, but this should probably be changed. I believe there are quite some downsides to doing it like this, especially considering how mounting this onto the shadow-dom and dom would be done.


type stateType = {
Copy link
Contributor

Choose a reason for hiding this comment

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

All types should be Capitalized.

Copy link
Contributor

Choose a reason for hiding this comment

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

I recommend setting up tslint and prettier in your workspace. If not, run tslint --fix -p . in the root of the frontend folder.

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

type ActionType = {
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: stateType, action: ActionType): stateType => {
switch (action.type) {
case 'clear':
return initialState;
case 'good':
return {
snax: (
<SnackBarContainer
clicker={state.clicker}
good={true}
content={state.content}
/>
),
content: state.content,
};
case 'bad':
return {
snax: (
<SnackBarContainer
clicker={state.clicker}
good={false}
content={state.content}
/>
),
content: state.content,
};
default:
return initialState;
}
};

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

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
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { ITransaction } from '../declarations/transaction';
import GlobalWrapper from '../helpers/GlobalWrapper';
import { navbarWidth } from '../styling/sizes';
import { theme } from '../styling/theme';
import SnackBarContainer from '../components/atoms/SnackBarContainer';

addDecorator(storyFn => <GlobalWrapper>{storyFn()}</GlobalWrapper>);

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"
/>
));