Conversation
|
Please create the PR pointing to your repo 😅 |
Alex-TH
left a comment
Alex-TH
left a comment
There was a problem hiding this comment.
Overall looks good, great work, just the main things that I would recommend to check are:
- Spacing in your files, seems like there's some inconsistency with the indentation and general spacing of tokens, try to run a linter and / or prettier and fix the rules.
- Since your using styling components to add the css for the app I recommend to not mix approaches and just stick with styled components.
- The responsive designs it's good I just saw that the heights of the cards are not uniform and it can make the list look weird.
About the acceptance criteria points I saw the 3 of them just for the styles as commented above try to stick with one approach. For the bonus points I didn't saw any error on console and the design is responsive.
| @@ -0,0 +1,51 @@ | |||
| // import logo from './logo.svg'; | |||
| // import './App.css'; | |||
There was a problem hiding this comment.
You can completely remove the commented code, no need to keep it
| display:grid; | ||
| gap:1rem; | ||
| grid-template-columns:repeat(auto-fit, minmax(300px, 1fr)) | ||
| ` |
There was a problem hiding this comment.
There is something weird with the indentation please take a look and if needed configure your editor to fix it automatically.
|
|
||
|
|
||
| const VideoList = js.items.filter(vid => (vid.id.kind === "youtube#video")).map((vid)=>( | ||
| <Cards key={vid.id.videoId} className="cards"> |
There was a problem hiding this comment.
is the cards class still needed? I saw that you're using styling components, if there is a style declaration for cards maybe you should move it to the styled block.
3 participants
No description provided.