-
Notifications
You must be signed in to change notification settings - Fork 52
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
Video Details View - Mini Challenge 3 #47
base: master
Are you sure you want to change the base?
Changes from all commits
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 |
---|---|---|
|
@@ -13,6 +13,7 @@ | |
|
||
# misc | ||
.DS_Store | ||
.env | ||
.env.local | ||
.env.development.local | ||
.env.test.local | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
|
||
> react-certification-2020@0.1.0 test | ||
> react-scripts test "--coverage" "--watchAll" | ||
|
||
------------------------------|----------|----------|----------|----------|-------------------| | ||
File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s | | ||
------------------------------|----------|----------|----------|----------|-------------------| | ||
All files | 84.21 | 50 | 88.89 | 82.35 | | | ||
src | 0 | 100 | 100 | 0 | | | ||
index.js | 0 | 100 | 100 | 0 | 7 | | ||
src/components/App | 100 | 100 | 100 | 100 | | | ||
App.component.jsx | 100 | 100 | 100 | 100 | | | ||
index.js | 0 | 0 | 0 | 0 | | | ||
src/components/Navbar | 71.43 | 25 | 66.67 | 66.67 | | | ||
Navbar.component.jsx | 60 | 25 | 50 | 60 | 17,18 | | ||
index.js | 0 | 0 | 0 | 0 | | | ||
styles.js | 100 | 100 | 100 | 100 | | | ||
src/components/VideoCard | 100 | 100 | 100 | 100 | | | ||
VideoCard.component.jsx | 100 | 100 | 100 | 100 | | | ||
index.js | 0 | 0 | 0 | 0 | | | ||
src/components/VideosCatalog | 100 | 100 | 100 | 100 | | | ||
VideosCatalog.component.jsx | 100 | 100 | 100 | 100 | | | ||
index.js | 0 | 0 | 0 | 0 | | | ||
src/views/Home | 100 | 100 | 100 | 100 | | | ||
Home.page.jsx | 100 | 100 | 100 | 100 | | | ||
index.js | 0 | 0 | 0 | 0 | | | ||
------------------------------|----------|----------|----------|----------|-------------------| | ||
|
||
|
||
[1B[J |
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import React from 'react'; | ||
import { render, screen } from '@testing-library/react'; | ||
|
||
import App from './App.component'; | ||
|
||
describe('App', () => { | ||
it('renders App component', () => { | ||
render(<App />); | ||
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. Hmm 🤔 , this test is expecting nothing, this would increase your coverage but not really testing anything else than it is rendering with no errors |
||
}); | ||
}); |
This file was deleted.
This file was deleted.
This file was deleted.
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,56 @@ | ||||||
import React from 'react'; | ||||||
import AppBar from '@material-ui/core/AppBar'; | ||||||
import Toolbar from '@material-ui/core/Toolbar'; | ||||||
import Typography from '@material-ui/core/Typography'; | ||||||
import Button from '@material-ui/core/Button'; | ||||||
import IconButton from '@material-ui/core/IconButton'; | ||||||
import InputBase from '@material-ui/core/InputBase'; | ||||||
import SearchIcon from '@material-ui/icons/Search'; | ||||||
import MenuIcon from '@material-ui/icons/Menu'; | ||||||
import { useStyles } from './styles'; | ||||||
import { Link, useHistory } from 'react-router-dom'; | ||||||
|
||||||
export default function Navbar(props){ | ||||||
const classes = useStyles(); | ||||||
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. you could destructure the properties and use them directly at the JSX code
Suggested change
|
||||||
const width = window.innerWidth; | ||||||
const history = useHistory(); | ||||||
|
||||||
function handleSearch(event){ | ||||||
if(event.key === 'Enter'){ | ||||||
history.push('/'); | ||||||
props.handleSearch(event.target.value) | ||||||
} | ||||||
} | ||||||
|
||||||
return ( | ||||||
<div className={classes.root}> | ||||||
<AppBar position="sticky" color="default"> | ||||||
<Toolbar> | ||||||
<IconButton edge="start" className={classes.menuButton} color="inherit" aria-label="menu"> | ||||||
<MenuIcon /> | ||||||
</IconButton> | ||||||
<Link to='/' className={classes.navbarTitle}> | ||||||
<Typography variant="h6" > | ||||||
{ width < 480 ? "RB" : "React Bootcamp" } | ||||||
</Typography> | ||||||
</Link> | ||||||
<div className={classes.search}> | ||||||
<div className={classes.searchIcon}> | ||||||
<SearchIcon /> | ||||||
</div> | ||||||
<InputBase | ||||||
laceholder="Search a Video…" | ||||||
onKeyPress={handleSearch} | ||||||
classes={{ | ||||||
root: classes.inputRoot, | ||||||
input: classes.inputInput, | ||||||
}} | ||||||
inputProps={{ 'aria-label': 'search' }} | ||||||
/> | ||||||
</div> | ||||||
<Button className={classes.loginBtn}>Login</Button> | ||||||
</Toolbar> | ||||||
</AppBar> | ||||||
</div> | ||||||
); | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import React from 'react'; | ||
import { render, screen } from '@testing-library/react'; | ||
|
||
import Navbar from './Navbar.component'; | ||
|
||
describe('Navbar', () => { | ||
it('renders Navbar component', () => { | ||
render(<Navbar />); | ||
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. same 😢 |
||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { default } from './Navbar.component'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
import { fade, makeStyles } from '@material-ui/core/styles'; | ||
|
||
const useStyles = makeStyles((theme) => ({ | ||
root: { | ||
flexGrow: 1, | ||
marginBottom: "30px" | ||
}, | ||
menuButton: { | ||
marginRight: theme.spacing(2), | ||
}, | ||
grow: { | ||
flexGrow: 1, | ||
}, | ||
search: { | ||
position: 'relative', | ||
borderRadius: theme.shape.borderRadius, | ||
backgroundColor: fade(theme.palette.common.white, 0.40), | ||
'&:hover': { | ||
backgroundColor: fade(theme.palette.common.white, 1), | ||
}, | ||
marginRight: theme.spacing(2), | ||
marginLeft: 0, | ||
width: '100%', | ||
[theme.breakpoints.up('sm')]: { | ||
marginLeft: theme.spacing(1), | ||
width: 'auto', | ||
}, | ||
}, | ||
searchIcon: { | ||
padding: theme.spacing(0, 2), | ||
height: '100%', | ||
position: 'absolute', | ||
pointerEvents: 'none', | ||
display: 'flex', | ||
alignItems: 'center', | ||
justifyContent: 'center', | ||
}, | ||
inputRoot: { | ||
color: 'inherit', | ||
}, | ||
inputInput: { | ||
padding: theme.spacing(1, 1, 1, 0), | ||
paddingLeft: `calc(1em + ${theme.spacing(4)}px)`, | ||
transition: theme.transitions.create('width'), | ||
width: '100%', | ||
[theme.breakpoints.up('sm')]: { | ||
width: '20ch', | ||
'&:focus': { | ||
width: '20ch', | ||
}, | ||
}, | ||
}, | ||
navbarTitle: { | ||
textTransform: 'uppercase', | ||
fontWeight: 'bold', | ||
fontSize: '24px', | ||
flexGrow: 1, | ||
textDecoration: 'none', | ||
color: '#fb5607' | ||
}, | ||
loginBtn: { | ||
background: 'linear-gradient(45deg, #2196F3 30%, #21CBF3 90%)', | ||
color: 'white', | ||
border: 0, | ||
borderRadius: 3, | ||
boxShadow: '0 3px 5px 2px rgba(33, 203, 243, .3)', | ||
fontWeight: 800 | ||
}, | ||
})); | ||
|
||
export { useStyles } |
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,39 @@ | ||||||
import React from 'react'; | ||||||
import Card from '@material-ui/core/Card'; | ||||||
import CardContent from '@material-ui/core/CardContent'; | ||||||
import CardMedia from '@material-ui/core/CardMedia'; | ||||||
import Typography from '@material-ui/core/Typography'; | ||||||
import { Link } from 'react-router-dom'; | ||||||
import { useStyles } from './styles'; | ||||||
|
||||||
export default function RelatedCard({videoInfo}) { | ||||||
|
||||||
console.log(videoInfo); | ||||||
|
||||||
const classes = useStyles(); | ||||||
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. you are not passing a theme object, so I think that's the reason you are missing more than several styles at your components. Do not want to try styled-components instead? |
||||||
const { title, channelTitle, thumbnails } = videoInfo.snippet; | ||||||
const { videoId } = videoInfo.id; | ||||||
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.
Suggested change
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. even better if you integrate it with the previous line |
||||||
|
||||||
return ( | ||||||
<Link to={`/${videoId}`} className={classes.link}> | ||||||
<Card className={classes.root}> | ||||||
<div className={classes.details}> | ||||||
<CardContent className={classes.content}> | ||||||
<Typography noWrap component="h5" variant="h5" className={classes.title}> | ||||||
{ title } | ||||||
</Typography> | ||||||
<Typography variant="subtitle1" color="textSecondary" className={classes.channel}> | ||||||
{ channelTitle } | ||||||
</Typography> | ||||||
</CardContent> | ||||||
</div> | ||||||
<CardMedia | ||||||
className={classes.cover} | ||||||
image={thumbnails.medium.url} | ||||||
title="Live from space album cover" | ||||||
/> | ||||||
</Card> | ||||||
</Link> | ||||||
|
||||||
); | ||||||
} |
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.
😎 good coverage so far