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

feat: favorites view and API integration #18

Merged
merged 3 commits into from
Dec 17, 2024
Merged

feat: favorites view and API integration #18

merged 3 commits into from
Dec 17, 2024

Conversation

SofiaCantero24
Copy link
Owner

What does this do?

Favorites screen and API

Simulator.Screen.Recording.-.iPhone.16.Pro.-.2024-12-10.at.11.43.02.mp4

Notion Ticket

Figma view

Why did you do this?

Who/what does this impact?

How did you test this?

Comment on lines 45 to 48
className={`mx-4 border ${
isFirstItem ? 'rounded-t-lg' : ''
} ${isLastItem ? 'rounded-b-lg' : ''}`}
>
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't forget to use twMerge

Comment on lines 76 to 78
className={`mx-4 border ${isFirstItem ? 'rounded-t-lg' : ''} ${
isLastItem ? 'rounded-b-lg' : ''
}`}
Copy link
Collaborator

Choose a reason for hiding this comment

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

here also, use twMerge

state === 'used' ? 'bg-restored' : 'bg-new'
}`}
>
<Text className={`text-xs font-semibold text-white`}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<Text className={`text-xs font-semibold text-white`}>
<Text className="text-xs font-semibold text-white">

Comment on lines 19 to 21
className={`mb-2 mt-1 w-12 items-center space-x-2 rounded-md px-2 py-1 ${
state === 'used' ? 'bg-restored' : 'bg-new'
}`}
Copy link
Collaborator

Choose a reason for hiding this comment

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

dont forget to use twMerge

Comment on lines 39 to 54
const { mutate: addFavorite } = useAddFavorite({
onSuccess: () => {
refetch();
},
onError: () => {
showErrorMessage('Something went wrong');
},
});
const { mutate: removeFavorite } = useRemoveFavorite({
onSuccess: () => {
refetch();
},
onError: () => {
showErrorMessage('Something went wrong');
},
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel this is too much responsability to the component to carry. Components should be mostly visual components and the logic should live in the parent

}: TitleAndFavoriteProps) => {
const { mutate: addFavorite } = useAddFavorite({
onSuccess: () => {
refetch();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I recommend you to read about queries invalidations: https://tanstack.com/query/v3/docs/framework/react/guides/invalidations-from-mutations so you don't need to care about manually refetching.

<Link
href={{
pathname: detailsRoute,
params: { id: id },
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
params: { id: id },
params: { id },

<Link
href={{
pathname: detailsRoute,
params: { id: id },
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
params: { id: id },
params: { id },

mutationKey: ['addFavorite'],
mutationFn: addFavorite,
onSuccess: (data, variables, context) => {
queryClient.invalidateQueries({ queryKey: ['products'] });
Copy link
Collaborator

Choose a reason for hiding this comment

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

Cool!

@SofiaCantero24 SofiaCantero24 merged commit f511213 into main Dec 17, 2024
6 checks passed
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.

3 participants