Create a snackbar to notify the user of important events#226
Create a snackbar to notify the user of important events#226
Conversation
…on closing button
| @@ -0,0 +1,23 @@ | |||
| import styled from 'styled-components'; | |||
| import { theme } from '../../styling/theme'; | |||
There was a problem hiding this comment.
Don't import the theme from file, this doesn't allow it to update if the theme changes.
| import { theme } from '../../styling/theme'; | ||
|
|
||
| export default styled.button` | ||
| background-color: ${theme.palette.primary.main}; |
There was a problem hiding this comment.
This should be
| background-color: ${theme.palette.primary.main}; | |
| background-color: ${props => props.theme.palette.primary.main}; |
| position: absolute; | ||
| right: 10px; | ||
| top: 1.2em; | ||
| color: ${theme.palette.primary.contrast}; |
| margin-left: 1em; | ||
|
|
||
| &:hover { | ||
| color: ${theme.palette.danger.main}; |
| import React from 'react'; | ||
| import styled from 'styled-components'; | ||
| import SnackBarCloseButton from './SnackBarCloseButton'; | ||
| import { theme } from '../../styling/theme'; |
There was a problem hiding this comment.
Here as well. Won't comment anymore on this as I think you get the gist ;)
| interval_type, | ||
| start_date: date, | ||
| template: { | ||
| if (!(description.length > 140) && !(notes.length > 140)) { |
There was a problem hiding this comment.
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.|
|
||
| interface ISnackBarProps { | ||
| className?: string; | ||
| clicker?: () => void; |
There was a problem hiding this comment.
Name should be updated, this doesn't make it very clear what it does. If it's a handler, use the appropriate type.
| } | ||
|
|
||
| interface IAction { | ||
| type: 'clear' | 'good' | 'bad'; |
There was a problem hiding this comment.
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 initialState = { snax: <div />, content: '' }; | ||
|
|
||
| interface IState { |
There was a problem hiding this comment.
| interface IState { | |
| interface ISnackStore { |
| type: 'clear' | 'good' | 'bad'; | ||
| } | ||
|
|
||
| const reducer = (state: IState, action: IAction): IState => { |
There was a problem hiding this comment.
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.
- Reducer should be moved to its own file.
- 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.).
- The
goodandbadreducer actions do the exact same thing except set thegoodprop differently. This means that it should probably be generalized, and you should make anaddToastaction that takescontentandvariant.
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.
There was a problem hiding this comment.
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:
- snack created; 'hello'
- user clicks; removed
- snack created; 'hello'
- timeout for first snack hits; removes second snack (3)
- timeout for second snack hits; does nothing
Implement snackbar feature when either adding transaction or trying to add transaction with name or description too long.
Closes #68
Closes #207