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 4: State management #92

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

Conversation

AntonioSalazar
Copy link

Mini_Challenge_4_testing_coverage

Site can be visited in here: https://youtube-clone-wizeline.netlify.app/

unknown added 30 commits July 18, 2021 00:54
…e Navbar styles file and the indes.jx file
installed node-sass, creating my own layout now
hamburguer menu, the input field for searching videos and the user
button
…ow the search will be available to other components
…iles, after the user enters a topic the input search form gets cleaned out
…ork when it reners the VideoDetails component
Copy link

@cehawz cehawz left a comment

Choose a reason for hiding this comment

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

Necesitas desacoplar la llamada al API de VideoList.provider.js

}

case 'REMOVE_FROM_FAVORITES':
const favoriteVideosFromLocalStorage = localStorage.getItem('favorite_videos');
Copy link

Choose a reason for hiding this comment

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

Este bloque de codigo de la 29-32 debe de estar en el action.

Copy link
Author

Choose a reason for hiding this comment

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

listo! Ya movi estas lineas de codigo :)

const history = useHistory()
useEffect(() => {
let mounted = true;
const getVideos = async () => {
Copy link

Choose a reason for hiding this comment

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

Este get videos debes de desacoplarlo al provider para que puedas hacer unit testing. Ademas de que esta funcionalidad deberia de estar definida en el hook de youtube.

Copy link
Author

Choose a reason for hiding this comment

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

@cehawz listo, ya pase esta funcionalidad al custom hook

unknown added 23 commits August 25, 2021 11:59
…Fetch, I have also refactored the VideoList Provider so it does not have the api call inside of it
…custom Hook useFetch, everything seems to be working now
…Fetch, I have also refactored the VideoList Provider so it does not have the api call inside of it
…custom Hook useFetch, everything seems to be working now
…e VideoListReducer to the VideoListProvider, I have also changed the favicon
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants