Skip to content
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

Mini challenge 3 #46

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

AlanCantabrana
Copy link

Captura de Pantalla 2021-04-02 a la(s) 17 57 57

Copy link

@PacoMojica PacoMojica left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Alan, I like what you have up to this point,

Acceptance Criteria

  • Videos in the Home View are fetched from the YouTube API and displayed correctly.
  • Users can search for YouTube videos using the search field on the header.
  • A video can be played from the Video Details View after clicking on it.
  • The video information and related videos list are displayed correctly on the Video Details View.
  • When a user clicks on a related video the video player, information and related videos list are updated.

Bonus Points

  • Custom Hooks for API calls are implemented and tested correctly.
  • Testing coverage is above 60%. (Please include a screenshot of the code coverage report in your PR description).

----------------------------|----------|----------|----------|----------|-------------------|
File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s |
----------------------------|----------|----------|----------|----------|-------------------|
All files | 89.09 | 80 | 92 | 89.81 | |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, the coverage percentage is high on all columns

import LoginPage from '../../pages/Login';
import NotFound from '../../pages/NotFound';
import SecretPage from '../../pages/Secret';
import Private from '../Private';
import Fortune from '../Fortune';
// import Fortune from '../Fortune';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can delete that code, I don't think you'll need it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants