-
Notifications
You must be signed in to change notification settings - Fork 52
Challenge 4 #57
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
base: master
Are you sure you want to change the base?
Challenge 4 #57
Changes from all commits
8cc4c26
d40b158
01ee58c
ecdb29e
f5f05c3
af685e7
17babce
c5cc39b
89c2e49
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| REACT_APP_API_YOUTUBE_KEY = AIzaSyAOsyZMq5msH04OZ9W7Y_fQX--TictP_v8 | ||
Large diffs are not rendered by default.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import React, { useReducer } from 'react'; | ||
| import NavBar from '../NavBar'; | ||
| import HomePage from '../../pages/Home'; | ||
| import SearchContext from '../../state/SearchContext'; | ||
| import SearchReducer from '../../state/SearchReducer'; | ||
| import LocalThemeProvider from '../../state/ThemeContext.provider'; | ||
|
|
||
| function App() { | ||
| // const [search, setSearch] = useState('Wizeline'); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I recommend removing all commented code so it won't be kept commented in the future. |
||
| const [state, dispatch] = useReducer(SearchReducer, { | ||
| search: 'Wizeline', | ||
| }); | ||
|
|
||
| const handleSearch = (e) => { | ||
| const searchValue = e.target.value; | ||
| dispatch({ | ||
| type: 'SEARCH_VIDEOS', | ||
| payload: { | ||
| search: searchValue, | ||
| }, | ||
| }); | ||
| }; | ||
|
|
||
| return ( | ||
| <div> | ||
| <LocalThemeProvider> | ||
| <SearchContext.Provider value={{ state, dispatch }}> | ||
| <NavBar search={handleSearch} /> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would use the context inside the NavBar itself instead of creating a function and then send it to the component. The prop drilling is not being removed even with the usage of a Context. One of the usages of the context is to remove prop drilling |
||
| <HomePage /> | ||
| </SearchContext.Provider> | ||
| </LocalThemeProvider> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| export default App; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| export { default } from './App.component'; | ||
| export { default } from './App'; |
This file was deleted.
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| import React from 'react'; | ||
| import styled from 'styled-components'; | ||
|
|
||
| const InputSearchStyled = styled.div` | ||
| margin-left: 50px; | ||
| `; | ||
|
|
||
| export default function InputSearch({ search }) { | ||
| return ( | ||
| <InputSearchStyled> | ||
| <input type="text" placeholder="Search" onChange={search} /> | ||
| </InputSearchStyled> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export { default } from './InputSearch'; |
This file was deleted.
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| import React from 'react'; | ||
| import { FcManager } from 'react-icons/fc'; | ||
| import styled from 'styled-components'; | ||
| import InputSearch from '../InputSearch'; | ||
|
|
||
| const NavBarStyled = styled.div` | ||
| background-color: ${(props) => props.theme.primary}; | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: space-between; | ||
| `; | ||
|
Comment on lines
+6
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I recommend taking out the style from the logic of the components |
||
|
|
||
| function NavBar({ search }) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also recommend using the context hook to obtain the search value instead of using prop drilling to obtain the value. |
||
| return ( | ||
| <NavBarStyled> | ||
| <InputSearch search={search} /> | ||
| <div> | ||
| <FcManager size="60" /> | ||
| </div> | ||
| </NavBarStyled> | ||
| ); | ||
| } | ||
|
|
||
| export default NavBar; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export { default } from './NavBar'; |
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| import React from 'react'; | ||
| import { Container, Title } from './VideoCard.styles'; | ||
|
|
||
| const VideoCard = ({ video, handleVideoSelected }) => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| console.log(`Hello;${video}`); | ||
| return ( | ||
| <Container> | ||
| <div role="button"> | ||
| <img alt={video.snippet.description} src={video.snippet.thumbnails.default.url} /> | ||
| <div> | ||
| <Title>{video.snippet.title}</Title> | ||
| </div> | ||
| </div> | ||
| <button type="button" onClick={() => handleVideoSelected(video)}> | ||
| View details | ||
| </button> | ||
| </Container> | ||
| ); | ||
| }; | ||
|
|
||
| export default VideoCard; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| import styled from 'styled-components'; | ||
|
|
||
| export const Container = styled.div` | ||
| border-bottom: 1px solid #cccccc; | ||
| border-radius: 5px; | ||
| padding: 20px; | ||
| text-align: left; | ||
| :hover { | ||
| background-color: ${(props) => props.theme.primary}; | ||
| color: ${(props) => props.theme.text}; | ||
| } | ||
| `; | ||
|
|
||
| export const Title = styled.div` | ||
| color: ${(props) => props.theme.textcolor}; | ||
| `; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export { default } from './VideoCard'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import React from 'react'; | ||
| import styled from 'styled-components'; | ||
|
|
||
| const VideoStyled = styled.div` | ||
| .video { | ||
| margin-top: 50px; | ||
| width: 100%; | ||
| height: 300px; | ||
| } | ||
| `; | ||
| const Title = styled.h4` | ||
| color: ${(props) => props.theme.textcolor}; | ||
| `; | ||
|
Comment on lines
+4
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I recommend to abstract always the CSS styles from the components |
||
|
|
||
| const VideoDetail = ({ video }) => { | ||
| if (!video) { | ||
| return ( | ||
| <div> | ||
| <Title>Select a video</Title> | ||
| </div> | ||
| ); | ||
| } | ||
| const videoSrc = `https://www.youtube.com/embed/${video.id.videoId}`; | ||
| return ( | ||
| <VideoStyled> | ||
| <iframe className="video" src={videoSrc} title="Video" /> | ||
|
|
||
| <div> | ||
| <Title>{video.snippet.title}</Title> | ||
| <Title>{video.snippet.description}</Title> | ||
| </div> | ||
| </VideoStyled> | ||
| ); | ||
| }; | ||
|
|
||
| export default VideoDetail; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export { default } from './VideoDetail'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| import React from 'react'; | ||
| import styled from 'styled-components'; | ||
| import VideoCard from '../VideoCard'; | ||
|
|
||
| const VideoListContainer = styled.div` | ||
| text-align: center; | ||
| `; | ||
|
Comment on lines
+5
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removing CSS styling from components |
||
|
|
||
| export default function VideoList({ videos, handleVideoSelected }) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of using prop drilling with |
||
| if (!videos) { | ||
| return <div />; | ||
| } | ||
|
|
||
| const renderedVideos = videos.map((video) => { | ||
| return ( | ||
| <VideoCard | ||
| key={Math.random()} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And instead of using a |
||
| video={video} | ||
| handleVideoSelected={handleVideoSelected} | ||
| /> | ||
| ); | ||
| }); | ||
| return <VideoListContainer>{renderedVideos}</VideoListContainer>; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also recommend putting the map itself inside the video.map(...) |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export { default } from './VideoList'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recommend to not keep track of this file, it is a good practice to create a .env file but also not keeping track of this file.