Skip to content

Mini challenge5#99

Open
BigOsvaap wants to merge 14 commits into
cvillanueva84:masterfrom
BigOsvaap:mini-challenge5
Open

Mini challenge5#99
BigOsvaap wants to merge 14 commits into
cvillanueva84:masterfrom
BigOsvaap:mini-challenge5

Conversation

@BigOsvaap

Copy link
Copy Markdown

No description provided.

@dianaatwizeline dianaatwizeline left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

good PR. My biggest advice is to move the dispatch calls as I mention in the comment

};

return (
<>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't think this empty fragment <> is necessary here

`;

export function CardVideoBottom({ title, description }) {
function trucateText(text) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this function can live outside CardVideoBottom so that it does not get redefined on every render

});
setButtonClicked(false);
} else {
dispatch({

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

As an optional challenge, try to move the dispatch calls to inside the FavoritesProvider so that way instead of exposing dispatch and need to remember the exact type of action, you expose more straightforward functions like addFavorite and deleteFavorite that take the payload as the only parameter

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