Mini - Challenge 5 #4
Conversation
- Add router - Delete missing files - create constant - fetch data changing approach - Move Card into a component instead of page folder
| overflow: hidden; | ||
| `; | ||
|
|
||
| const StyledLink = styled(Link)` |
There was a problem hiding this comment.
Good use of the extend styles feature from styled-components!
| `; | ||
|
|
||
| const Styled = { | ||
| Link: StyledLink, |
There was a problem hiding this comment.
You can directly add this to the export statement.
|
|
||
| function ModalLogin({ show }) { | ||
| const { data, dispatch } = useData(); | ||
| const [userName, setUserName] = useState(undefined); |
There was a problem hiding this comment.
Leaving the useState() blank without any param is the save as saying useState(undefined). What I do in the case of an input is initializing the state to an empty string like useState('').
| function ModalLogin({ show }) { | ||
| const { data, dispatch } = useData(); | ||
| const [userName, setUserName] = useState(undefined); | ||
| const [password, setPassword] = useState(undefined); |
| } | ||
|
|
||
| function handlerLogin() { | ||
| console.log(data); |
There was a problem hiding this comment.
Remember to delete this for production or real-life scenarios 😝 .
| return ( | ||
| <div> | ||
| {data.loginUser ? ( | ||
| '' |
There was a problem hiding this comment.
In this case, instead of an empty string, it's better to use null.
| } | ||
|
|
||
| return ( | ||
| <div> |
There was a problem hiding this comment.
Instead of using an extra div here to suffice to React constraints of wrapping code in one container, you can use React Fragmens so that we don't necessarily add an extra node to the DOM.
You can either say:
<React.Fragment>
{your code here}
</React.Fragment>Or you can import Fragment from React:
import { Fragment } from 'react';
...
<Fragment>
{your code here}
</Fragment>Or take advantage the syntactic sugar and use:
<>
{your code here}
</>|
|
||
| export default function Routes() { | ||
| return ( | ||
| <> |
There was a problem hiding this comment.
A Fragment <></> is not necessary here since Switch is wrapping the whole block of code/nodes.
| JSON.stringify(Array.from(favorites.entries())) | ||
| ); | ||
| } else { | ||
| console.log( |
There was a problem hiding this comment.
In a real-life scenario it would make sense to also handle errors in the UI so the user is aware of it!
There was a problem hiding this comment.
Great work @j-polanco !
Solid work here! The only comments that I made were minor things. I know it is a smaller thing and it works as it is, but maybe we can improve the styles of the page too 🎨..
Other than that:
- Good use of
styled-componentsand its features and APIs. 💅🏼 - Good use of React hooks and good to see you made your own. 🔥
- Good understanding of state management. ⌛
- Good practices overall (naming, reusing code, etc.) 👍🏼
2 participants
Acceptance Criteria
https://gist.github.com/jparciga/2c2375835ea5e26ba3bc2bd6a30f0ee5
Changes
Demo
https://priceless-wescoff-41c153.netlify.app/