Skip to content

fixed #348 #396

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
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fixed #348 #396

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f357b76
fixed #348
Saksham1387 Apr 7, 2025
9c2d392
converted to component LabelsDisplay
Saksham1387 Apr 8, 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
43 changes: 43 additions & 0 deletions apps/web/components/email-list/EmailListItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ import { Button } from "@/components/ui/button";
import { findCtaLink } from "@/utils/parse/parseHtml.client";
import { ErrorBoundary } from "@/components/ErrorBoundary";
import { internalDateToDate } from "@/utils/date";
import { Badge } from "@/components/ui/badge";
import { isDefined } from "@/utils/types";
import { useLabels } from "@/hooks/useLabels";

export const EmailListItem = forwardRef(
(
Expand All @@ -45,13 +48,25 @@ export const EmailListItem = forwardRef(
ref: ForwardedRef<HTMLLIElement>,
) => {
const { thread, splitView, onSelected } = props;
const { userLabels } = useLabels();

const lastMessage = thread.messages?.[thread.messages.length - 1];

const isUnread = useMemo(() => {
return lastMessage?.labelIds?.includes("UNREAD");
}, [lastMessage?.labelIds]);

const labelsToDisplay = useMemo(() => {
const labelIds = lastMessage?.labelIds;
return labelIds
?.map((id) => {
const label = userLabels[Number(id)];
Copy link
Owner

Choose a reason for hiding this comment

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

why do we convert to number?

Copy link
Author

Choose a reason for hiding this comment

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

otherwise it will be of type any and typescript complains

Copy link
Owner

Choose a reason for hiding this comment

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

doesn't make sense. something hacky going on here.

if (!label) return null;
return { id, name: label.name };
})
.filter(isDefined);
}, [lastMessage?.labelIds, userLabels]);
Comment on lines +59 to +68
Copy link
Contributor

@coderabbitai coderabbitai bot Apr 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix the label lookup logic.

The current implementation treats userLabels as an object with numeric keys, but according to the useLabels hook implementation, it returns an array. You should use array methods like find to locate labels by ID.

const labelsToDisplay = useMemo(() => {
  const labelIds = lastMessage?.labelIds;
  return labelIds
    ?.map((id) => {
-      const label = userLabels[Number(id)];
+      const label = userLabels.find(label => label.id === id);
      if (!label) return null;
      return { id, name: label.name };
    })
    .filter(isDefined);
}, [lastMessage?.labelIds, userLabels]);
📝 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 labelsToDisplay = useMemo(() => {
const labelIds = lastMessage?.labelIds;
return labelIds
?.map((id) => {
const label = userLabels[Number(id)];
if (!label) return null;
return { id, name: label.name };
})
.filter(isDefined);
}, [lastMessage?.labelIds, userLabels]);
const labelsToDisplay = useMemo(() => {
const labelIds = lastMessage?.labelIds;
return labelIds
?.map((id) => {
const label = userLabels.find(label => label.id === id);
if (!label) return null;
return { id, name: label.name };
})
.filter(isDefined);
}, [lastMessage?.labelIds, userLabels]);

Copy link
Owner

Choose a reason for hiding this comment

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

see this

Copy link
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

@elie222 I see the CodeRabbit suggestion about the label lookup logic. Let me investigate the actual implementation of the useLabels hook to verify whether the current approach or the suggested change is correct.

🏁 Script executed:

#!/bin/bash
# Find the useLabels hook implementation
fd -e tsx -e ts -e js useLabels

Length of output: 61

🏁 Script executed:

#!/bin/bash
# Search for the useLabels hook definition
ast-grep --pattern 'export function useLabels() {
  $$$
}'

Length of output: 878

🏁 Script executed:

#!/bin/bash
# Alternative search for useLabels hook
rg "useLabels" -A 20 -B 5

Length of output: 28833

@elie222 You're right to point this out! After examining the useLabels hook implementation, I can confirm that CodeRabbit's suggestion is correct.

The current implementation in the diff is incorrect because:

  1. userLabels is an array, not an object: The useLabels() hook returns userLabels as an array: data?.labels?.filter(isUserLabel).sort(sortLabels) || []

  2. Label IDs are strings, not numeric indices: Gmail label IDs are strings like "UNREAD", "INBOX", user-defined label IDs, etc. Converting them to numbers with Number(id) and using them as array indices doesn't make sense.

  3. Incorrect lookup method: userLabels[Number(id)] tries to access array elements by numeric index, but we need to find labels by their id property.

CodeRabbit's suggested fix is correct:

const label = userLabels.find(label => label.id === id);

This properly searches through the array to find a label object whose id property matches the label ID from the email message.

The current code would likely return undefined for most labels, causing the label badges not to display properly.

🏁 Script executed:

#!/bin/bash
# Let's also examine the file content to see the complete useLabels hook implementation
cat apps/web/hooks/useLabels.ts

Length of output: 2124

Perfect! Now I can see the complete picture. The useLabels hook implementation confirms that CodeRabbit's suggestion is absolutely correct.

Here's the definitive analysis:

The Problem

The current code:

const label = userLabels[Number(id)];

This is wrong because:

  1. userLabels is an array of UserLabel objects, not an object with numeric keys
  2. Label IDs are strings (like "UNREAD", "INBOX", or custom label IDs like "Label_123")
  3. Array indices have no relation to label IDs - the array is just a filtered and sorted list of user labels

The Correct Solution

CodeRabbit's suggestion is the proper way to find labels:

const label = userLabels.find(label => label.id === id);

This correctly:

  • Searches through the userLabels array
  • Finds the label object whose id property matches the email's label ID
  • Returns the matching UserLabel object or undefined if not found

Impact

The current incorrect implementation would cause:

  • Label badges not displaying for most emails
  • Potential runtime errors or unexpected behavior
  • Labels being silently ignored rather than shown to users

CodeRabbit's suggestion should definitely be applied to fix the label display functionality in the email list.

🧬 Code Graph Analysis Results

Relevant snippets for understanding apps/web/components/email-list/EmailListItem.tsx

File: apps/web/utils/types.ts (lines 8-10)

export function isDefined<T>(value: T | undefined | null): value is T {
  return value !== undefined && value !== null;
}
  • Utility function used in EmailListItem to filter out undefined or null values from arrays, ensuring type safety.

File: apps/web/components/email-list/types.ts (lines 5-11)

export type Thread = {
  id: FullThread["id"];
  messages: FullThread["messages"];
  snippet: FullThread["snippet"];
  plan: FullThread["plan"];
  category: FullThread["category"];
};
  • Defines the Thread type used as a prop in EmailListItem. It includes email thread metadata such as messages, snippet, plan, and category.

File: apps/web/utils/email.ts (lines 6-13 and 74-81)

export function extractNameFromEmail(email: string) {
  if (!email) return "";
  const firstPart = email.split("<")[0]?.trim();
  if (firstPart) return firstPart;
  const secondPart = email.split("<")?.[1]?.trim();
  if (secondPart) return secondPart.split(">")[0];
  return email;
}

export function participant(
  message: { headers: Pick<ParsedMessage["headers"], "from" | "to"> },
  userEmail: string,
) {
  if (!userEmail) return message.headers.from;
  if (message.headers.from.includes(userEmail)) return message.headers.to;
  return message.headers.from;
}
  • extractNameFromEmail extracts a display name from an email string.
  • participant determines the other participant in the email thread relative to the user's email address.
  • Both are used in EmailListItem to display sender/recipient names.

File: apps/web/utils/gmail/decode.ts (implied usage)

  • The decodeSnippet function is imported and used to decode email snippet text for display.
  • (No snippet provided, but relevant as it processes the snippet content.)

File: apps/web/utils/parse/parseHtml.client.ts (implied usage)

  • The findCtaLink function is imported and used to find a call-to-action link in the email's HTML content.
  • (No snippet provided, but relevant for rendering CTA buttons.)

File: apps/web/hooks/useLabels.ts (lines 37-52)

export function useLabels() {
  const { data, isLoading, error, mutate } =
    useSWR<LabelsResponse>("/api/google/labels");

  const userLabels = useMemo(
    () => data?.labels?.filter(isUserLabel).sort(sortLabels) || [],
    [data?.labels],
  );

  return {
    userLabels,
    isLoading,
    error,
    mutate,
  };
}
  • Custom hook used in EmailListItem to fetch and provide user labels for emails.
  • Labels are used to display badges next to email subjects.

File: apps/web/utils/date.ts (lines 46-53)

export function internalDateToDate(internalDate?: string | null): Date {
  if (!internalDate) return new Date();

  const date = new Date(+internalDate);
  if (Number.isNaN(date.getTime())) return new Date();

  return date;
}
  • Converts internal date string (likely from Gmail API) to JavaScript Date object.
  • Used in EmailListItem to display the email date.

File: apps/web/components/email-list/EmailDate.tsx (lines 3-9)

export function EmailDate(props: { date: Date }) {
  return (
    <div className="flex-shrink-0 text-sm font-medium leading-5 text-muted-foreground">
      {formatShortDate(props.date)}
    </div>
  );
}
  • Component used in EmailListItem to render the formatted date of the email.

File: apps/web/components/ActionButtons.tsx (lines 14-85)

export function ActionButtons({
  threadId,
  onArchive,
  onPlanAiAction,
  isPlanning,
  refetch,
  shadow,
}: {
  threadId: string;
  isPlanning: boolean;
  shadow?: boolean;
  onPlanAiAction: () => void;
  onArchive: () => void;
  refetch: (threadId?: string) => void;
}) {
  const session = useSession();
  const email = session.data?.user.email;

  const openInGmail = useCallback(() => {
    // open in gmail
    const url = getGmailUrl(threadId, email);
    window.open(url, "_blank");
  }, [threadId, email]);

  const [isTrashing, setIsTrashing] = useState(false);

  // TODO lift this up to the parent component to be consistent / to support bulk trash
  // TODO show loading toast
  const onTrash = useCallback(async () => {
    setIsTrashing(true);
    await onTrashThread(threadId);
    refetch(threadId);
    setIsTrashing(false);
  }, [threadId, refetch]);

  const buttons = useMemo(
    () => [
      {
        tooltip: "Open in Gmail",
        onClick: openInGmail,
        icon: <ExternalLinkIcon className="size-4" aria-hidden="true" />,
      },
      {
        tooltip: "Process with assistant",
        onClick: onPlanAiAction,
        icon: isPlanning ? (
          <LoadingMiniSpinner />
        ) : (
          <SparklesIcon className="size-4" aria-hidden="true" />
        ),
      },
      {
        tooltip: "Archive",
        onClick: onArchive,
        icon: <ArchiveIcon className="size-4" aria-hidden="true" />,
      },
      // may remove later
      {
        tooltip: "Delete",
        onClick: onTrash,
        icon: isTrashing ? (
          <LoadingMiniSpinner />
        ) : (
          <Trash2Icon className="size-4" aria-hidden="true" />
        ),
      },
    ],
    [onTrash, isTrashing, onArchive, onPlanAiAction, isPlanning, openInGmail],
  );

  return <ButtonGroup buttons={buttons} shadow={shadow} />;
}
  • Component rendering action buttons (Open in Gmail, Process with assistant, Archive, Delete) for each email thread.
  • Used inside EmailListItem for the right-side action buttons.

File: apps/web/components/ui/badge.tsx (lines 30-47)

export const Badge = forwardRef(
  (
    props: { children: React.ReactNode; color: Color; className?: string },
    ref: ForwardedRef<HTMLSpanElement | null>,
  ) => {
    const { color, className, ...rest } = props;

    return (
      <span
        ref={ref}
        {...rest}
        className={cn(badgeVariants({ color, className }))}
      >
        {props.children}
      </span>
    );
  },
);
  • UI component used to render badges, such as labels in LabelsDisplay inside EmailListItem.

File: apps/web/components/ui/button.tsx (line 99)

Button
  • The Button component is imported and used in EmailListItem to render clickable buttons, including the CTA button.

Summary

The above snippets provide context on:

  • Types and utilities for email threads and labels.
  • Helper functions for extracting participant names and decoding snippets.
  • UI components for badges, buttons, dates, and action buttons.
  • Hooks for fetching user labels.
  • Date conversion utilities.

These are all directly used or referenced in EmailListItem.tsx and are relevant to understanding its implementation and behavior.


const preventPropagation = useCallback(
(e: React.MouseEvent | React.KeyboardEvent) => e.stopPropagation(),
[],
Expand Down Expand Up @@ -138,6 +153,9 @@ export const EmailListItem = forwardRef(
)}
<div className="ml-2 min-w-0 overflow-hidden text-foreground">
{lastMessage.headers.subject}
{labelsToDisplay && labelsToDisplay.length > 0 && (
<LabelsDisplay labels={labelsToDisplay} />
)}
</div>
<div className="ml-4 mr-6 flex flex-1 items-center overflow-hidden truncate font-normal leading-5 text-muted-foreground">
{decodedSnippet}
Expand Down Expand Up @@ -195,6 +213,9 @@ export const EmailListItem = forwardRef(
<div className="mt-1.5 whitespace-nowrap text-sm leading-6">
<div className="min-w-0 overflow-hidden font-medium text-foreground">
{lastMessage.headers.subject}
{labelsToDisplay && labelsToDisplay.length > 0 && (
<LabelsDisplay labels={labelsToDisplay} />
)}
</div>
<div className="mr-6 mt-0.5 flex flex-1 items-center overflow-hidden truncate pl-1 font-normal leading-5 text-muted-foreground">
{decodedSnippet}
Expand All @@ -216,3 +237,25 @@ export const EmailListItem = forwardRef(
);

EmailListItem.displayName = "EmailListItem";

type Label = {
id: string;
name: string;
};

type LabelBadgesProps = {
labels: Label[] | undefined;
};

function LabelsDisplay({ labels }: LabelBadgesProps) {
if (!labels || labels.length === 0) return null;
return (
<span className="ml-2 inline-flex flex-wrap items-center gap-1">
{labels.map((label) => (
<Badge variant="secondary" key={label.id} className="text-xs">
{label.name}
</Badge>
))}
</span>
);
}