Mini-Challenge 1: Core Concepts and Styling #30
Conversation
erickwize
left a comment
erickwize
left a comment
There was a problem hiding this comment.
Comments (Mini-challenge 1 & 2)
I see that your app works as expected nice work! But the code is all wrapped in only three components, in my personal opinion, we could extract more than three components here (i.e. Input, SideBar, Icons).
Please for the next deliverables provide a file with the results of running the coverage of your tests.
Also the next deliverable please create a PR into your own repo, so we could merge it back into your code once the PR is approved.
Acceptance Criteria (Mini-challenge 1)
- The header is rendered correctly.
- A list of videos obtained from a mock file is displayed in the Home View.
- CSS styles are applied correctly using the styled-components approach.
Bonus Points (Mini-challenge 1)
- No warnings or errors are logged in the browser console.
- The UI is responsive
Acceptance Criteria (Mini-challenge 2)
- Meaningful test cases were implemented for components and sub-routines logic.
- All the test cases were successful.
Bonus Points (Mini-challenge 2)
- Test coverage is above 60%.
| const ContainerHeader = styled.header` | ||
| display: flex; | ||
| align-items: center; | ||
| position: fixed; | ||
| top:0; | ||
| width: 100%; | ||
| height: 65px; | ||
| background-color: #ACDEDF; | ||
|
|
||
| `; | ||
|
|
||
| const MenuButton = styled.button` | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| width: 55px; | ||
| height: 55px; | ||
| padding: 10px; | ||
| background-color: transparent; | ||
| border:none; | ||
| font-size: 1.3rem; | ||
| `; | ||
|
|
||
| const InputSearch = styled.input` | ||
| align-items: center; | ||
| padding: 10px; | ||
| background-color: transparent; | ||
| box-sizing: content-box; | ||
| padding: 10px; | ||
| box-sizing: content-box; | ||
| border: 1px solid #bbc1bf; | ||
| font-size: 1.3rem; | ||
| background: #0000; | ||
| border-radius: 3px; | ||
| height: 1rem; | ||
| font-size: 1rem; | ||
| padding-left: 30px; | ||
| `; | ||
|
|
||
| const DivSearch = styled.div` | ||
| cursor: text; | ||
| display: inline-flex; | ||
| position: relative; | ||
| font-size: 1rem; | ||
| align-items: center; | ||
| height: 100%; | ||
| justify-content: center; | ||
| `; | ||
|
|
||
| const DivIcon = styled.div` | ||
| justify-content: center; | ||
| color: #fff; | ||
| position: relative; | ||
| left: 30px; | ||
| height: 100%; | ||
| align-items: center; | ||
| display: flex; | ||
| font-size: 1.3rem; | ||
| `; |
There was a problem hiding this comment.
Please extract all the styles into different files depending of the component. You have various components mixed on the same file. I would create i.e. a file called SearchBar.styled.js, inside the file I inserted all the related styles for the SearchBar, export all the styles, and in another file SearchBar.component.jsx use them.
You have all the components in the Header component. React's good practice is to create small components, so you could reuse them in different places of your app, if you keep them in the same it is difficult to keep track where the component is defined and how to reuse it.
| const DivFlex = styled.div` | ||
| flex-grow: 1; | ||
| `; | ||
|
|
||
| const ToggleSwitch = styled.label` | ||
| position: relative; | ||
| display: inline-block; | ||
| width: 60px; | ||
| height: 30px; | ||
| `; | ||
|
|
||
| const Slider = styled.span` | ||
| position: absolute; | ||
| cursor: pointer; | ||
| border-radius: 30px; | ||
| top: 0; | ||
| left: 0; | ||
| right: 0; | ||
| bottom: 0; | ||
| background-color: #ccc; | ||
| -webkit-transition: .4s; | ||
| transition: .4s; | ||
| &::before { | ||
| position: absolute; | ||
| content: ""; | ||
| height: 24px; | ||
| width: 24px; | ||
| left: 4px; | ||
| bottom: 4px; | ||
| background-color: white; | ||
| -webkit-transition: .4s; | ||
| transition: .4s; | ||
| border-radius: 50%; | ||
| } | ||
| `; | ||
|
|
||
| const CheckboxSwitch = styled.input` | ||
| background-color: #b39541; | ||
| opacity: 0; | ||
| width: 0px; | ||
| height: 0px; | ||
| &:checked + ${Slider} { | ||
| background: #b39541; | ||
| &::before { | ||
| transform: translateX(24px); | ||
| } | ||
| } | ||
| `; | ||
| const Span = styled.span` | ||
| color: #b39541; | ||
| font-size: 0.8rem; | ||
| padding: 10px; | ||
| `; | ||
|
|
||
| const Avatar = styled.div` | ||
| background-color: #b39541; | ||
| border-radius: 999999px; | ||
| margin: 1%; | ||
| height: 45px; | ||
| width: 45px; | ||
| opacity: 0.5; | ||
| color: #fff; | ||
| display: inline-flex; | ||
| font-size: 1.6rem; | ||
| align-items: center; | ||
| justify-content: center; | ||
| `; |
There was a problem hiding this comment.
The same CSS extraction comment here
| const Container = styled.div` | ||
| width: 323px; | ||
| height: 400px; | ||
| margin: 10px; | ||
| max-width: 100%; | ||
| display: inline-grid; | ||
| overflow: hidden; | ||
| border: 1px solid #eee; | ||
| max-width: 100%; | ||
| padding: 10px; | ||
| `; | ||
|
|
||
| const Description = styled.span` | ||
| font-size: .8rem; | ||
| line-height: 1.2rem; | ||
| display: inline-block; | ||
| `; | ||
|
|
||
| const ContainerImg = styled.div` | ||
| text-align:center; | ||
| height: 180px; | ||
| overflow: hidden; | ||
| `; | ||
|
|
||
| const Image = styled.img` | ||
| max-width:100%; | ||
| height: 100%; | ||
| `; | ||
|
|
||
| const Title = styled.h3` | ||
| font-size: 1rem; | ||
| line-height: 1.2rem; | ||
| display: inline-block; | ||
| `; |
There was a problem hiding this comment.
Same CSS extraction comment here
| const ContainerVideos = styled.div` | ||
| max-width: 100%; | ||
| margin: 0 5%; | ||
| `; |
There was a problem hiding this comment.
Same CSS extraction comment here
2 participants
No description provided.