Skip to content

View emails in side panel#304

Merged
elie222 merged 10 commits intomainfrom
email-side-panel
Jan 11, 2025
Merged

View emails in side panel#304
elie222 merged 10 commits intomainfrom
email-side-panel

Conversation

@elie222
Copy link
Owner

@elie222 elie222 commented Jan 11, 2025

Summary by CodeRabbit

Release Notes

  • New Features

    • Added EmailViewer component to allow viewing email threads.
    • Introduced ViewEmailButton for quick email thread access.
    • Enhanced email-related components with thread ID support.
  • Improvements

    • Streamlined email state management using new useDisplayedEmail hook.
    • Updated UI labels for better clarity in cold email and category sections.
    • Simplified button and component interactions.
  • UI Changes

    • Replaced modal-based forms with direct form components.
    • Updated tab and button styling in various sections.

@vercel
Copy link

vercel bot commented Jan 11, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
inbox-zero ✅ Ready (Inspect) Visit Preview Jan 11, 2025 8:42pm

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 11, 2025

Warning

Rate limit exceeded

@elie222 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 23 minutes and 4 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 56f0914 and 5915174.

📒 Files selected for processing (1)
  • apps/web/components/EmailViewer.tsx (1 hunks)

Walkthrough

The pull request introduces a comprehensive set of changes across multiple files in the web application, focusing on enhancing email-related functionality. The primary modifications include adding a new EmailViewer component, introducing the useDisplayedEmail and useThread hooks, and updating various components to support thread-based email viewing. The changes streamline email interaction by providing a consistent way to view emails across different sections of the application, with a particular emphasis on adding threadId to components and implementing a side panel for email display.

Changes

File Change Summary
apps/web/app/(app)/layout.tsx Added EmailViewer component to AppLayout
apps/web/components/EmailViewer.tsx New component for displaying email threads
apps/web/components/ViewEmailButton.tsx New button component to trigger email viewing
apps/web/hooks/useDisplayedEmail.ts New hook for managing displayed email state
apps/web/hooks/useThread.ts New hook for fetching thread data
apps/web/app/(app)/automation/ExecutedRulesTable.tsx Enhanced EmailCell with threadId and added ViewEmailButton
apps/web/app/(app)/automation/History.tsx Updated EmailCell to include threadId prop
apps/web/app/(app)/automation/Pending.tsx Added threadId prop to EmailCell and modified query state key
apps/web/app/(app)/automation/ProcessRules.tsx Added threadId to TestRulesMessage component
apps/web/app/(app)/cold-email-blocker/ColdEmailSettings.tsx Replaced ColdEmailPromptModal with ColdEmailPromptForm
apps/web/app/(app)/cold-email-blocker/TestRules.tsx Added threadId to TestRulesMessage and modified button styling
apps/web/components/email-list/EmailList.tsx Transitioned to useQueryState for managing query parameters
apps/web/components/email-list/EmailPanel.tsx Updated function signatures for improved prop access
apps/web/components/ui/sheet.tsx Introduced SheetOverlayProps interface for overlay management
apps/web/store/email.ts Removed selectedEmailAtom for state management changes

Sequence Diagram

sequenceDiagram
    participant User
    participant ViewEmailButton
    participant EmailViewer
    participant useDisplayedEmail
    participant useThread
    
    User->>ViewEmailButton: Click to view email
    ViewEmailButton->>useDisplayedEmail: Call showEmail(threadId)
    useDisplayedEmail-->>EmailViewer: Update thread state
    EmailViewer->>useThread: Fetch thread data
    useThread-->>EmailViewer: Return thread details
    EmailViewer->>User: Display email thread
Loading

Possibly related PRs

Poem

🐰 Hop, hop, through code so bright,
Emails dancing left and right,
ThreadId joins the merry chase,
Viewer opens with such grace,
Rabbit's magic makes emails gleam! 🌟


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (8)
apps/web/hooks/useThread.ts (1)

7-10: Consider adding SWR configuration options

The hook implementation is clean, but could benefit from additional SWR options for better user experience:

  • Error retry configuration
  • Revalidation intervals
  • Loading timeout
 export function useThread({ id }: ThreadQuery) {
   const url = `/api/google/threads/${id}`;
-  return useSWR<ThreadResponse>(url);
+  return useSWR<ThreadResponse>(url, {
+    revalidateOnFocus: false,
+    errorRetryCount: 2,
+    loadingTimeout: 3000,
+  });
 }
apps/web/components/ViewEmailButton.tsx (1)

19-31: Add loading state feedback

The button should show a loading state while the email is being fetched to provide better user feedback.

+ import { Loader2Icon } from "lucide-react";
+ import { useState } from "react";

 return (
   <Tooltip content="View email">
     <Button
       variant="outline"
       size={size || "icon"}
-      onClick={() => showEmail({ threadId, messageId })}
+      onClick={() => {
+        setIsLoading(true);
+        showEmail({ threadId, messageId });
+      }}
       className={className}
+      disabled={isLoading}
     >
-      <MailIcon className="h-4 w-4" />
+      {isLoading ? (
+        <Loader2Icon className="h-4 w-4 animate-spin" />
+      ) : (
+        <MailIcon className="h-4 w-4" />
+      )}
       <span className="sr-only">View email</span>
     </Button>
   </Tooltip>
 );
apps/web/components/EmailViewer.tsx (2)

15-27: Add keyboard shortcut for closing the panel

Consider adding the Escape key handler for better user experience.

+ import { useEffect } from "react";

 return (
   <Sheet open={!!threadId} onOpenChange={hideEmail}>
+    {!!threadId && (
+      <useEffect(() => {
+        const handleEsc = (e: KeyboardEvent) => {
+          if (e.key === "Escape") hideEmail();
+        };
+        window.addEventListener("keydown", handleEsc);
+        return () => window.removeEventListener("keydown", handleEsc);
+      }, [hideEmail])
+    )}
     <SheetContent
       side="right"
       size="5xl"
       className="overflow-y-auto p-0"
       overlay="transparent"
     >
       {threadId && <EmailContent threadId={threadId} />}
     </SheetContent>
   </Sheet>
 );

17-22: Configure Sheet transition animation

The side panel would benefit from smooth transition animations.

 <SheetContent
   side="right"
   size="5xl"
-  className="overflow-y-auto p-0"
+  className="overflow-y-auto p-0 transition-transform duration-300 ease-in-out"
   overlay="transparent"
 >
apps/web/app/(app)/cold-email-blocker/ColdEmailPromptForm.tsx (1)

64-64: Fix typo and improve readability in explain text.

There's a missing space after "needs." in the explain text. Consider restructuring for better readability:

-        label="Prompt to classify cold emails"
-        explainText="Adjust to your needs.Use a similar style for best results. Delete your prompt to revert to the default prompt."
+        label="Prompt to classify cold emails"
+        explainText="Adjust to your needs. Use a similar style for best results. Delete your prompt to revert to the default."

Also applies to: 67-67

apps/web/app/(app)/cold-email-blocker/ColdEmailSettings.tsx (1)

29-34: LGTM! Improved component architecture.

The changes enhance the component structure by:

  1. Placing TestRules logically at the top
  2. Using mutate for state updates instead of refetch
  3. Simplifying the form implementation
apps/web/app/(app)/automation/ExecutedRulesTable.tsx (1)

146-152: Add accessibility attributes to Gmail link.

The external link should have an aria-label for better screen reader support.

 <Link
   href={getGmailUrl(messageId, userEmail)}
   target="_blank"
+  aria-label="Open in Gmail (opens in new tab)"
   className="ml-2 text-gray-700 hover:text-gray-900"
 >
   <ExternalLinkIcon className="h-4 w-4" />
 </Link>
apps/web/components/ui/sheet.tsx (1)

48-48: Consider using CSS variables for width values.

The hardcoded width values (w-3/4) in the sheet variants could be made more maintainable using CSS variables.

-        left: "inset-y-0 left-0 h-full w-3/4 border-r data-[state=closed]:slide-out-to-left data-[state=open]:slide-in-from-left",
+        left: "inset-y-0 left-0 h-full w-[var(--sheet-width,75%)] border-r data-[state=closed]:slide-out-to-left data-[state=open]:slide-in-from-left",
-          "inset-y-0 right-0 h-full w-3/4  border-l data-[state=closed]:slide-out-to-right data-[state=open]:slide-in-from-right",
+          "inset-y-0 right-0 h-full w-[var(--sheet-width,75%)]  border-l data-[state=closed]:slide-out-to-right data-[state=open]:slide-in-from-right",

Also applies to: 50-50

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 53fc47c and 855239a.

📒 Files selected for processing (22)
  • apps/web/app/(app)/automation/ExecutedRulesTable.tsx (3 hunks)
  • apps/web/app/(app)/automation/History.tsx (2 hunks)
  • apps/web/app/(app)/automation/Pending.tsx (2 hunks)
  • apps/web/app/(app)/automation/ProcessRules.tsx (1 hunks)
  • apps/web/app/(app)/cold-email-blocker/ColdEmailPromptForm.tsx (2 hunks)
  • apps/web/app/(app)/cold-email-blocker/ColdEmailRejected.tsx (1 hunks)
  • apps/web/app/(app)/cold-email-blocker/ColdEmailSettings.tsx (2 hunks)
  • apps/web/app/(app)/cold-email-blocker/TestRules.tsx (2 hunks)
  • apps/web/app/(app)/cold-email-blocker/TestRulesMessage.tsx (2 hunks)
  • apps/web/app/(app)/cold-email-blocker/page.tsx (1 hunks)
  • apps/web/app/(app)/layout.tsx (2 hunks)
  • apps/web/app/(app)/smart-categories/setup/SetUpCategories.tsx (1 hunks)
  • apps/web/components/CommandK.tsx (3 hunks)
  • apps/web/components/EmailViewer.tsx (1 hunks)
  • apps/web/components/GroupedTable.tsx (2 hunks)
  • apps/web/components/ViewEmailButton.tsx (1 hunks)
  • apps/web/components/email-list/EmailList.tsx (6 hunks)
  • apps/web/components/email-list/EmailPanel.tsx (1 hunks)
  • apps/web/components/ui/sheet.tsx (3 hunks)
  • apps/web/hooks/useDisplayedEmail.ts (1 hunks)
  • apps/web/hooks/useThread.ts (1 hunks)
  • apps/web/store/email.ts (0 hunks)
💤 Files with no reviewable changes (1)
  • apps/web/store/email.ts
🔇 Additional comments (25)
apps/web/app/(app)/automation/ProcessRules.tsx (1)

261-262: LGTM! Verify TestRulesMessage component interface.

The addition of threadId and messageId props aligns with the PR objective of implementing email viewing in the side panel.

Let's verify the TestRulesMessage component interface accepts these props:

✅ Verification successful

Props match TestRulesMessage component interface

The addition of threadId and messageId props is verified to match the TestRulesMessage component interface and follows consistent naming conventions throughout the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify TestRulesMessage component accepts threadId and messageId props

# Check TestRulesMessage component definition
ast-grep --pattern 'type $_ = {
  $$$
  threadId: $_
  messageId: $_
  $$$
}'

# Check for consistent prop naming across related components
rg -A 2 'threadId.*messageId|messageId.*threadId' --type typescript

Length of output: 184


Script:

#!/bin/bash
# Search for TestRulesMessage component definition and props
ast-grep --pattern 'interface TestRulesMessage'
ast-grep --pattern 'type TestRulesMessage'
ast-grep --pattern 'const TestRulesMessage'

# Search for TestRulesMessage usage and props
rg "TestRulesMessage.*?(\{|<)" --type ts --type tsx -A 3

# Search for threadId and messageId props in ts/tsx files
rg "threadId|messageId" --type ts --type tsx -A 2

Length of output: 307


Script:

#!/bin/bash
# Find TestRulesMessage-related files
fd TestRulesMessage

# Search for component definition and usage
rg "TestRulesMessage" -A 5

# Search for props pattern
rg "type.*Props.*=|interface.*Props|threadId.*:|messageId.*:" -A 3

Length of output: 53403

apps/web/app/(app)/cold-email-blocker/TestRulesMessage.tsx (1)

8-8: LGTM! Clean integration of email viewing capability.

The ViewEmailButton is well-integrated alongside the existing email actions, with proper TypeScript types for the new threadId prop.

Also applies to: 15-15, 22-22, 36-41

apps/web/app/(app)/cold-email-blocker/page.tsx (1)

22-22: LGTM! Improved tab label clarity.

The label change from "Not Cold" to "Marked Not Cold" is more explicit and better describes the tab's purpose.

apps/web/app/(app)/layout.tsx (1)

18-18: LGTM! Clean integration of EmailViewer.

The EmailViewer component is well-placed for implementing the side panel functionality. However, let's verify error handling coverage.

Let's check if EmailViewer is covered by error boundaries:

Also applies to: 47-47

apps/web/app/(app)/cold-email-blocker/ColdEmailPromptForm.tsx (1)

Line range hint 16-46: LGTM! Solid form implementation.

The form implementation is well-structured with:

  • Proper zod validation
  • Type-safe props and handlers
  • Good error handling with user feedback
apps/web/app/(app)/cold-email-blocker/ColdEmailRejected.tsx (1)

115-116: LGTM! Improved user messaging.

The updated alert text is more clear and informative, helping users better understand when emails will appear in this view.

apps/web/components/CommandK.tsx (1)

27-27: LGTM! Improved email state management.

Good transition from atom-based to hook-based state management, providing more centralized control over email display state.

Also applies to: 33-40

apps/web/app/(app)/automation/ExecutedRulesTable.tsx (1)

23-34: LGTM! Enhanced email cell functionality.

Good addition of thread context and email viewing capabilities through the ViewEmailButton integration.

Also applies to: 38-38, 55-55

apps/web/components/ui/sheet.tsx (3)

18-21: Well-structured interface addition for overlay customization.

The new SheetOverlayProps interface with the optional overlay prop provides good flexibility for styling customization.


52-54: Good addition of size variants with sensible defaults.

The size variants provide good flexibility, and setting "sm" as the default is a reasonable choice.

Also applies to: 59-59


70-97: Well-structured component refactor with proper prop handling.

The SheetContent component's refactor properly handles the new overlay and size props while maintaining good TypeScript type safety.

apps/web/app/(app)/cold-email-blocker/TestRules.tsx (1)

178-179: Good addition of thread context to email messages.

The addition of threadId and messageId props enhances the email viewing functionality.

apps/web/app/(app)/smart-categories/setup/SetUpCategories.tsx (1)

50-50: Consistent query parameter naming convention.

The change from "categoryName" to "category-name" aligns with kebab-case naming convention for URL parameters.

apps/web/app/(app)/automation/Pending.tsx (2)

38-39: Consistent query parameter naming convention.

The change from "ruleId" to "rule-id" maintains consistency with kebab-case URL parameter naming.


182-183: Enhanced email context with thread information.

The addition of threadId and messageId props to EmailCell improves the email viewing experience.

apps/web/components/email-list/EmailPanel.tsx (1)

94-94: LGTM! Good job on making the EmailThread component reusable.

The export of the EmailThread component allows it to be reused in the side panel implementation, promoting code reusability.

apps/web/components/GroupedTable.tsx (2)

62-62: LGTM! Good job on integrating the ViewEmailButton.

The import of the ViewEmailButton component aligns with the PR objective of viewing emails in a side panel.


544-546: LGTM! Good job on implementing the view email functionality.

The ViewEmailButton is correctly integrated into the table cell with the required threadId and messageId props.

apps/web/components/email-list/EmailList.tsx (5)

4-4: LGTM! Good job on using useQueryState for state management.

Using useQueryState from nuqs for state management is a better approach as it makes the email view state shareable and bookmarkable via URL parameters.


182-185: LGTM! Good job on implementing the side panel state.

The implementation correctly manages the side panel state using useQueryState for openThreadId and provides a clean way to close the panel.


189-190: LGTM! Good job on memoizing the opened row.

The useMemo hook is correctly used to memoize the opened row based on openThreadId and threads dependencies.


485-486: LGTM! Good job on handling the email opening logic.

The implementation correctly:

  1. Checks if a thread is already open
  2. Updates the openThreadId state
  3. Updates the UI accordingly with opened and splitView props

Also applies to: 506-506, 510-510


547-558: LGTM! Good job on implementing the side panel rendering.

The side panel is correctly rendered when both openThreadId and openedRow are available, with proper prop passing to the EmailPanel component.

apps/web/app/(app)/automation/History.tsx (2)

27-27: LGTM! Good job on importing the useDisplayedEmail hook.

The import of the useDisplayedEmail hook aligns with the PR objective of managing email display state.


99-101: LGTM! Good job on enhancing the EmailCell component.

The EmailCell component is correctly enhanced with threadId and messageId props to support email viewing functionality.

Comment on lines +8 to +19
const showEmail = useCallback(
(
options: {
threadId: string;
messageId?: string;
} | null,
) => {
setThreadId(options?.threadId ?? null);
setMessageId(options?.messageId ?? null);
},
[setMessageId, setThreadId],
);
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add input validation for thread and message IDs

The showEmail callback should validate the input IDs before updating the URL state to prevent XSS or invalid states.

 const showEmail = useCallback(
   (
     options: {
       threadId: string;
       messageId?: string;
     } | null,
   ) => {
+    if (options?.threadId && !/^[a-zA-Z0-9-_]+$/.test(options.threadId)) {
+      console.error('Invalid threadId format');
+      return;
+    }
+    if (options?.messageId && !/^[a-zA-Z0-9-_]+$/.test(options.messageId)) {
+      console.error('Invalid messageId format');
+      return;
+    }
     setThreadId(options?.threadId ?? null);
     setMessageId(options?.messageId ?? null);
   },
   [setMessageId, setThreadId],
 );
📝 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 showEmail = useCallback(
(
options: {
threadId: string;
messageId?: string;
} | null,
) => {
setThreadId(options?.threadId ?? null);
setMessageId(options?.messageId ?? null);
},
[setMessageId, setThreadId],
);
const showEmail = useCallback(
(
options: {
threadId: string;
messageId?: string;
} | null,
) => {
if (options?.threadId && !/^[a-zA-Z0-9-_]+$/.test(options.threadId)) {
console.error('Invalid threadId format');
return;
}
if (options?.messageId && !/^[a-zA-Z0-9-_]+$/.test(options.messageId)) {
console.error('Invalid messageId format');
return;
}
setThreadId(options?.threadId ?? null);
setMessageId(options?.messageId ?? null);
},
[setMessageId, setThreadId],
);

Comment on lines 29 to 37
function EmailContent({ threadId }: { threadId: string }) {
const { data, isLoading, error, mutate } = useThread({ id: threadId });

return (
<LoadingContent loading={isLoading} error={error}>
{data && <EmailThread messages={data.thread.messages} refetch={mutate} />}
</LoadingContent>
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Implement error boundary for EmailContent

Add an error boundary to gracefully handle runtime errors in the email content rendering.

+ import { ErrorBoundary } from "react-error-boundary";

 function EmailContent({ threadId }: { threadId: string }) {
   const { data, isLoading, error, mutate } = useThread({ id: threadId });

   return (
+    <ErrorBoundary
+      FallbackComponent={({ error }) => (
+        <div className="p-4">
+          <h3 className="text-destructive">Error loading email</h3>
+          <p>{error.message}</p>
+        </div>
+      )}
+    >
       <LoadingContent loading={isLoading} error={error}>
         {data && <EmailThread messages={data.thread.messages} refetch={mutate} />}
       </LoadingContent>
+    </ErrorBoundary>
   );
 }
📝 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
function EmailContent({ threadId }: { threadId: string }) {
const { data, isLoading, error, mutate } = useThread({ id: threadId });
return (
<LoadingContent loading={isLoading} error={error}>
{data && <EmailThread messages={data.thread.messages} refetch={mutate} />}
</LoadingContent>
);
}
import { ErrorBoundary } from "react-error-boundary";
function EmailContent({ threadId }: { threadId: string }) {
const { data, isLoading, error, mutate } = useThread({ id: threadId });
return (
<ErrorBoundary
FallbackComponent={({ error }) => (
<div className="p-4">
<h3 className="text-destructive">Error loading email</h3>
<p>{error.message}</p>
</div>
)}
>
<LoadingContent loading={isLoading} error={error}>
{data && <EmailThread messages={data.thread.messages} refetch={mutate} />}
</LoadingContent>
</ErrorBoundary>
);
}

Comment on lines +33 to +40
if (threadId) {
const threadIds = [threadId];
archiveEmails(threadIds, undefined, () => {
return refreshEmailList?.refetch({ removedThreadIds: threadIds });
});
setSelectedEmail(undefined);
showEmail(null);
}
}, [refreshEmailList, selectedEmail, setSelectedEmail]);
}, [refreshEmailList, threadId, showEmail]);
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for archive operation.

Consider adding error handling and loading state for the archive operation to improve user feedback:

 const onArchive = React.useCallback(() => {
   if (threadId) {
     const threadIds = [threadId];
+    try {
       archiveEmails(threadIds, undefined, () => {
         return refreshEmailList?.refetch({ removedThreadIds: threadIds });
       });
       showEmail(null);
+    } catch (error) {
+      console.error('Archive operation failed:', error);
+      // Show error toast to user
+    }
   }
 }, [refreshEmailList, threadId, showEmail]);
📝 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
if (threadId) {
const threadIds = [threadId];
archiveEmails(threadIds, undefined, () => {
return refreshEmailList?.refetch({ removedThreadIds: threadIds });
});
setSelectedEmail(undefined);
showEmail(null);
}
}, [refreshEmailList, selectedEmail, setSelectedEmail]);
}, [refreshEmailList, threadId, showEmail]);
if (threadId) {
const threadIds = [threadId];
try {
archiveEmails(threadIds, undefined, () => {
return refreshEmailList?.refetch({ removedThreadIds: threadIds });
});
showEmail(null);
} catch (error) {
console.error('Archive operation failed:', error);
// Show error toast to user
}
}
}, [refreshEmailList, threadId, showEmail]);

Comment on lines +36 to 39
<Button type="button">
<BookOpenCheckIcon className="mr-2 h-4 w-4" />
Test
</Button>
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Consider maintaining consistent button styling

The file shows inconsistent button styling for similar actions:

  • Main "Test" button has no variant
  • Form submit button has no variant
  • Content row "Test" button uses color="white"

Since all these buttons serve similar testing purposes, they should maintain consistent styling for better visual coherence.

🔗 Analysis chain

Consider maintaining consistent button styling.

The removal of the variant prop might affect visual consistency with other buttons in the application.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for button variant usage patterns
rg -A 1 '<Button\s+[^>]*variant="outline"' --type tsx

Length of output: 83


Script:

#!/bin/bash
# Search for Button components with variants in TypeScript/React files
rg -t typescript -t javascriptreact -t typescriptreact '<Button.*variant=' -A 1

# Also check for general Button usage patterns
rg -t typescript -t javascriptreact -t typescriptreact '<Button\s+[^>]*>' -A 1

Length of output: 232


Script:

#!/bin/bash
# Find all TypeScript/React files and search for Button components
fd -e tsx -e ts -e jsx -e js --exec rg -l '<Button.*variant=' {}

# Get content of the specific file we're reviewing
cat "apps/web/app/(app)/cold-email-blocker/TestRules.tsx"

Length of output: 9580


Script:

#!/bin/bash
# Check the original file this was copied from
cat "apps/web/app/(app)/automation/TestRules.tsx"

Length of output: 129

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
apps/web/components/EmailViewer.tsx (2)

38-38: Document the reasoning for disabled reply button.

The showReplyButton={false} prop is hardcoded without clear context. Consider:

  1. Adding a comment explaining why replies are disabled in the side panel
  2. Making it configurable through component props if there's no specific requirement
- showReplyButton={false}
+ // Disable reply button in side panel to avoid UI clutter
+ showReplyButton={false}

29-43: Consider memoizing EmailContent for performance.

Since EmailContent is a child component that depends only on threadId, consider wrapping it with React.memo to prevent unnecessary re-renders when parent state changes.

-function EmailContent({ threadId }: { threadId: string }) {
+const EmailContent = React.memo(function EmailContent({ threadId }: { threadId: string }) {
   const { data, isLoading, error, mutate } = useThread({ id: threadId });

   return (
     <LoadingContent loading={isLoading} error={error}>
       {data && (
         <EmailThread
           messages={data.thread.messages}
           refetch={mutate}
           showReplyButton={false}
         />
       )}
     </LoadingContent>
   );
-}
+});
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 855239a and 56f0914.

📒 Files selected for processing (2)
  • apps/web/components/EmailViewer.tsx (1 hunks)
  • apps/web/components/email-list/EmailPanel.tsx (8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/web/components/email-list/EmailPanel.tsx
🔇 Additional comments (3)
apps/web/components/EmailViewer.tsx (3)

1-9: LGTM! Dependencies are well-organized.

All necessary imports are present and properly structured for the component's functionality.


29-43: Add error boundary for robust error handling.

The component should implement an error boundary as suggested in the previous review.


16-22: Consider accessibility and responsive design implications.

The Sheet component's configuration raises a few concerns:

  1. Using overlay="transparent" might affect accessibility by not providing sufficient visual feedback for modal state
  2. The fixed size "5xl" might need adjustment for different screen sizes

Let's verify the responsive behavior:

@elie222 elie222 merged commit 00f9065 into main Jan 11, 2025
3 of 4 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Feb 6, 2025
@coderabbitai coderabbitai bot mentioned this pull request Feb 22, 2025
@elie222 elie222 deleted the email-side-panel branch December 18, 2025 23:01
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.

1 participant

Comments