Skip to content

enhancement: Add Like Button to Itinerary Detail Page #182

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.

Sign up for GitHub

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

Open
mmalkindi wants to merge 2 commits into
base: development
Choose a base branch
from
Open

enhancement: Add Like Button to Itinerary Detail Page #182

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4bd571a
feat: refactor Itinerary detail page header buttons
mmalkindi May 5, 2025
8e6f5e5
feat: add like button to itinerary detail page header
mmalkindi May 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 54 additions & 29 deletions src/modules/DetailItineraryModule/module-elements/ItineraryHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Button } from '@/components/ui/button'
import { useAuthContext } from '@/contexts/AuthContext'
import { Share2, X } from 'lucide-react'
import { Heart, Share2, X } from 'lucide-react'
import Link from 'next/link'
import { useEffect, useRef, useState } from 'react'
import {
Expand Down Expand Up @@ -33,6 +33,7 @@ export const ItineraryHeader = ({
const [emailInput, setEmailInput] = useState('')
const [emails, setEmails] = useState<string[]>([])
const inputRef = useRef<HTMLInputElement>(null)
const [isLiked, setIsLiked] = useState(false)
Copy link

@coderabbitai coderabbitai bot May 5, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Missing initialization of isLiked state.

The isLiked state is initialized to false, but there's no logic to set its initial value based on whether the user has already liked the itinerary. This could lead to an inconsistent UI state where an already-liked itinerary appears unliked when the page loads.

Consider adding a useEffect to fetch the initial liked status:

  const [isLiked, setIsLiked] = useState(false)

+ useEffect(() => {
+   // Fetch whether the current user has liked this itinerary
+   const checkLikedStatus = async () => {
+     try {
+       const response = await customFetch(`/itineraries/${data.id}/liked`, {
+         method: 'GET',
+         credentials: 'include',
+       });
+       if (response.success) {
+         setIsLiked(response.data.isLiked);
+       }
+     } catch (error) {
+       // Silently fail - default to unliked state
+       console.error('Error checking liked status:', error);
+     }
+   };
+   
+   if (user && user.id !== data.userId) {
+     void checkLikedStatus();
+   }
+ }, [data.id, user]);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const [isLiked, setIsLiked] = useState(false)
const [isLiked, setIsLiked] = useState(false)
useEffect(() => {
// Fetch whether the current user has liked this itinerary
const checkLikedStatus = async () => {
try {
const response = await customFetch(
`/itineraries/${data.id}/liked`,
{
method: 'GET',
credentials: 'include',
}
);
if (response.success) {
setIsLiked(response.data.isLiked);
}
} catch (error) {
// Silently fail - default to unliked state
console.error('Error checking liked status:', error);
}
};
if (user && user.id !== data.userId) {
void checkLikedStatus();
}
}, [data.id, user]);

Copy link
Contributor

@matthewhjt matthewhjt May 5, 2025

Choose a reason for hiding this comment

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

Halo, saya coba implementasi kode ini di local saya, sepertinya kamu lupa init value isLiked dengan benar. Buttonnya akan selalu tidak liked, meskipun sudah pernah dilike sebelumnya.

Sepertinya untuk memperbaiki ini tidak hanya harus mengubah kode frontend, tetapi juga mengubah implementasi backend untuk menginclude informasi apakah itinerarynya sudah dilike user atau belum.


useEffect(() => {
if (typeof window !== 'undefined') {
Expand Down Expand Up @@ -166,6 +167,22 @@ export const ItineraryHeader = ({
}
}

const toggleSaveItinerary = async () => {
try {
const response = await customFetch(`/itineraries/${data.id}/save`, {
method: isLiked === true ? 'DELETE' : 'POST',
credentials: 'include',
})

if (!response.success) throw Error(response.message)
setIsLiked(!isLiked)
} catch (error) {
toast.error(
error instanceof Error ? error.message : 'An unexpected error occurred'
)
}
}
Comment on lines +170 to +184
Copy link

@coderabbitai coderabbitai bot May 5, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Inconsistent error handling pattern.

The error handling in toggleSaveItinerary uses !response.success to check for errors, while other API calls in this file (like duplicateItinerary and removeUser) check the HTTP status code using response.statusCode.

For better consistency with the rest of the codebase, consider using the same error handling pattern:

  const toggleSaveItinerary = async () => {
    try {
      const response = await customFetch(`/itineraries/${data.id}/save`, {
        method: isLiked === true ? 'DELETE' : 'POST',
        credentials: 'include',
      })

-     if (!response.success) throw Error(response.message)
+     if (response.statusCode !== 200 && response.statusCode !== 201) throw Error(response.message)
      setIsLiked(!isLiked)
    } catch (error) {
      toast.error(
        error instanceof Error ? error.message : 'An unexpected error occurred'
      )
    }
  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const toggleSaveItinerary = async () => {
try {
const response = await customFetch(`/itineraries/${data.id}/save`, {
method: isLiked === true ? 'DELETE' : 'POST',
credentials: 'include',
})
if (!response.success) throw Error(response.message)
setIsLiked(!isLiked)
} catch (error) {
toast.error(
error instanceof Error ? error.message : 'An unexpected error occurred'
)
}
}
const toggleSaveItinerary = async () => {
try {
const response = await customFetch(`/itineraries/${data.id}/save`, {
method: isLiked === true ? 'DELETE' : 'POST',
credentials: 'include',
})
if (response.statusCode !== 200 && response.statusCode !== 201) throw Error(response.message)
setIsLiked(!isLiked)
} catch (error) {
toast.error(
error instanceof Error ? error.message : 'An unexpected error occurred'
)
}
}


const renderAcceptedUsers = () => {
if (isLoading) {
return <div className="text-center py-4">Memuat...</div>
Expand Down Expand Up @@ -295,18 +312,18 @@ export const ItineraryHeader = ({
)}
</div>
</div>
{user?.id === data.userId && (
<div className="absolute top-2 right-2 sm:top-4 sm:right-4 z-10 flex gap-2">
<Button
type="button"
size="sm"
variant="ghost"
className="bg-white text-black rounded-xl shadow"
onClick={openInviteDialog}
>
<Share2 className="w-6 h-6 text-[#004080]" />
</Button>
{user?.id === data.userId && (
<div className="absolute top-2 right-2 sm:top-4 sm:right-4 z-10 flex gap-2">
{user?.id === data.userId ? (
<>
<Button
type="button"
size="sm"
variant="ghost"
className="bg-white text-black rounded-xl shadow"
onClick={openInviteDialog}
>
<Share2 className="w-6 h-6 text-[#004080]" />
</Button>
<Link
href={contingencyId ? `${contingencyId}/edit` : `${data.id}/edit`}
>
Expand All @@ -319,22 +336,30 @@ export const ItineraryHeader = ({
Edit
</Button>
</Link>
)}
</div>
)}
{user?.id !== data.userId && (
<div className="absolute top-2 right-2 sm:top-4 sm:right-4 z-10 flex">
<Button
onClick={duplicateItinerary}
size="sm"
type="button"
variant="ghost"
className="bg-white text-[#004080] rounded-xl shadow"
>
Duplikasi dan Edit
</Button>
</div>
)}
</>
) : (
<>
<Button
type="button"
size="sm"
variant="ghost"
className="bg-white text-black rounded-xl shadow"
onClick={toggleSaveItinerary}
>
<Heart className="w-6 h-6 text-[#004080]" />
</Button>
Comment on lines +348 to +350
Copy link

@coderabbitai coderabbitai bot May 5, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add visual state indication to the heart icon.

The heart icon doesn't visually indicate whether the itinerary is liked or not. Users won't be able to tell the current state at a glance.

Add visual differentiation to the heart icon based on the isLiked state:

  <Button
    type="button"
    size="sm"
    variant="ghost"
    className="bg-white text-black rounded-xl shadow"
    onClick={toggleSaveItinerary}
+   aria-label={isLiked ? "Unlike itinerary" : "Like itinerary"}
  >
-   <Heart className="w-6 h-6 text-[#004080]" />
+   <Heart className={`w-6 h-6 ${isLiked ? "fill-[#004080] text-[#004080]" : "text-[#004080]"}`} />
  </Button>

This will fill the heart icon when the itinerary is liked, providing clear visual feedback to users, and also adds an appropriate aria-label for accessibility.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
>
<Heart className="w-6 h-6 text-[#004080]" />
</Button>
<Button
type="button"
size="sm"
variant="ghost"
className="bg-white text-black rounded-xl shadow"
onClick={toggleSaveItinerary}
aria-label={isLiked ? "Unlike itinerary" : "Like itinerary"}
>
<Heart
className={`w-6 h-6 ${
isLiked
? "fill-[#004080] text-[#004080]"
: "text-[#004080]"
}`}
/>
</Button>

Comment on lines +342 to +350
Copy link
Contributor

@matthewhjt matthewhjt May 5, 2025

Choose a reason for hiding this comment

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

Sebaiknya button likenya menggunakan komponen LikesButtton yang dipakai di Itinerary Card saja. Di sana sudah ada logic untuk toggle dan fetching ke backend. Implementasi kamu yang sekarang tidak membedakan apakah itinerarynya sudah dilike atau belum. Gambar hatinya tetap kosong meskipun berhasil melike. Selain itu, tidak ada penanda juga kalau proses like berhasil (kalau di LikesButton misalnya sudah ada logic untuk mengeluarkan toast apakah proses like berhasil atau tidak)

<Button
onClick={duplicateItinerary}
size="sm"
type="button"
variant="ghost"
className="bg-white text-[#004080] rounded-xl shadow"
>
Duplikasi dan Edit
</Button>
</>
)}
</div>

<Dialog
open={showModal || showInviteDialog}
Expand Down