Skip to content

Charts redesign#995

Merged
elie222 merged 91 commits intoelie222:mainfrom
jshwrnr:feat/charts-redesign
Nov 26, 2025
Merged

Charts redesign#995
elie222 merged 91 commits intoelie222:mainfrom
jshwrnr:feat/charts-redesign

Conversation

@jshwrnr
Copy link
Collaborator

@jshwrnr jshwrnr commented Nov 20, 2025

Summary by CodeRabbit

  • New Features

    • Added tabbed bar chart visualizations for enhanced data exploration in analytics.
    • Improved date range selection with relative date options and semantic labels.
  • Enhancements

    • Reorganized analytics dashboard controls for clearer interface layout.
    • Enhanced progress indicators with tooltip information display.
    • Updated filtering options with improved visual indicators.
    • Refined button styling and dropdown interactions.

✏️ Tip: You can customize this high-level summary in your review settings.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 1 file (reviewed changes from recent commits).

Prompt for AI agents (all 1 issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="apps/web/app/(app)/[emailAccountId]/mail/BetaBanner.tsx">

<violation number="1" location="apps/web/app/(app)/[emailAccountId]/mail/BetaBanner.tsx:13">
Removing the `onClose` handler means the beta banner can no longer be dismissed, so it permanently shows despite the stored `bannerVisible` flag.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

Copy link

@jit-ci jit-ci bot left a comment

Choose a reason for hiding this comment

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

❌ Jit security check failed to run.

#jit_bypass_commit in this PR to bypass, Jit Admin privileges required.

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

♻️ Duplicate comments (1)
apps/web/app/(app)/[emailAccountId]/mail/BetaBanner.tsx (1)

11-17: Beta banner is no longer dismissible; bannerVisible state is now effectively dead

With the new <Banner> usage, there is no onClose or other interaction wired to setBannerVisible, so users can no longer dismiss the beta banner. The useLocalStorage state now only honors whatever value was already in storage, but cannot be changed from the UI:

  • New users will always see the banner (since the default is true), with no way to hide it.
  • Existing users who previously dismissed it will still have it hidden, but likewise cannot toggle it back.

This matches the earlier review note about removing the onClose handler making the banner permanently visible despite the stored bannerVisible flag.

Consider either:

  • Reintroducing a dismiss affordance (e.g., extend Banner to accept an optional onClose and render a close button, then call setBannerVisible(false) here), or
  • If the intent is a permanently visible banner, remove the useLocalStorage state entirely to avoid unused state and confusion.

Example local change if you reintroduce onClose on Banner:

-  if (bannerVisible && typeof window !== "undefined")
-    return (
-      <Banner title="Beta">
-        Mail is currently in beta. It is not intended to be a full replacement
-        for your email client yet.
-      </Banner>
-    );
+  if (bannerVisible && typeof window !== "undefined")
+    return (
+      <Banner title="Beta" onClose={() => setBannerVisible(false)}>
+        Mail is currently in beta. It is not intended to be a full replacement
+        for your email client yet.
+      </Banner>
+    );

(Plus a matching optional onClose prop and close button in apps/web/components/Banner.tsx.)

🧹 Nitpick comments (1)
apps/web/components/List.tsx (1)

9-14: Align onSelect typing with its usage

onSelect is typed as a required callback, but the click handler calls it via onSelect?.(item), implying it might be undefined. It’s better to align types and runtime behavior:

  • If onSelect should always be provided (most common for a List control), drop the optional chaining:
-  onSelect: (item: ListItem) => void;
+  onSelect: (item: ListItem) => void;-            onClick={() => onSelect?.(item)}
+            onClick={() => onSelect(item)}
  • If you actually want to allow Lists without a selection handler, make the prop optional instead:
-  onSelect: (item: ListItem) => void;
+  onSelect?: (item: ListItem) => void;

Also applies to: 30-30

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5fd4e82 and 4907514.

📒 Files selected for processing (7)
  • apps/web/app/(app)/[emailAccountId]/mail/BetaBanner.tsx (1 hunks)
  • apps/web/app/(app)/[emailAccountId]/stats/BarListCard.tsx (1 hunks)
  • apps/web/components/List.tsx (1 hunks)
  • apps/web/components/StatsCard.tsx (1 hunks)
  • apps/web/components/TabSelect.tsx (2 hunks)
  • apps/web/components/charts/DomainIcon.tsx (1 hunks)
  • apps/web/components/charts/HorizontalBarChart.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • apps/web/components/StatsCard.tsx
  • apps/web/components/charts/HorizontalBarChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/BarListCard.tsx
  • apps/web/components/charts/DomainIcon.tsx
  • apps/web/components/TabSelect.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/app/(app)/[emailAccountId]/mail/BetaBanner.tsx (1)
apps/web/components/Banner.tsx (1)
  • Banner (8-24)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: cubic · AI code reviewer
  • GitHub Check: Jit Security
  • GitHub Check: test

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 2 files (reviewed changes from recent commits).

Prompt for AI agents (all 1 issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="apps/web/components/charts/HorizontalBarChart.tsx">

<violation number="1" location="apps/web/components/charts/HorizontalBarChart.tsx:52">
Only the linked state received a dark-mode text color, so non-link rows keep `text-gray-900` and become unreadable against the new dark gradient. Also add a `dark:text-…` color to the `&lt;span&gt;` fallback so every item remains legible in dark mode.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

? "noopener noreferrer"
: undefined
}
className="text-sm text-gray-900 dark:text-gray-100 truncate block z-10 relative hover:underline"
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 25, 2025

Choose a reason for hiding this comment

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

Only the linked state received a dark-mode text color, so non-link rows keep text-gray-900 and become unreadable against the new dark gradient. Also add a dark:text-… color to the <span> fallback so every item remains legible in dark mode.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/components/charts/HorizontalBarChart.tsx, line 52:

<comment>Only the linked state received a dark-mode text color, so non-link rows keep `text-gray-900` and become unreadable against the new dark gradient. Also add a `dark:text-…` color to the `&lt;span&gt;` fallback so every item remains legible in dark mode.</comment>

<file context>
@@ -49,7 +49,7 @@ export function HorizontalBarChart({
                           : undefined
                       }
-                      className=&quot;text-sm text-gray-900 truncate block z-10 relative hover:underline&quot;
+                      className=&quot;text-sm text-gray-900 dark:text-gray-100 truncate block z-10 relative hover:underline&quot;
                     &gt;
                       {item.name}
</file context>
Fix with Cubic

Copy link

@jit-ci jit-ci bot left a comment

Choose a reason for hiding this comment

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

❌ Jit security check failed to run.

#jit_bypass_commit in this PR to bypass, Jit Admin privileges required.

Copy link

@jit-ci jit-ci bot left a comment

Choose a reason for hiding this comment

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

❌ Jit security check failed to run.

#jit_bypass_commit in this PR to bypass, Jit Admin privileges required.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 5 files (reviewed changes from recent commits).

Prompt for AI agents (all 1 issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx">

<violation number="1" location="apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx:111">
`BarChart` always parses the x-axis field as a date, so rendering it with `xAxisKey=&quot;group&quot;` (rule names) causes `new Date(ruleName)` to throw whenever a tooltip renders. This breaks the chart on hover; use a component that supports categorical axes or adapt the data to real dates.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR


<TabsContent value="bar">
<TabsContent value="bar" className="mt-4">
<BarChart
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 25, 2025

Choose a reason for hiding this comment

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

BarChart always parses the x-axis field as a date, so rendering it with xAxisKey="group" (rule names) causes new Date(ruleName) to throw whenever a tooltip renders. This breaks the chart on hover; use a component that supports categorical axes or adapt the data to real dates.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx, line 111:

<comment>`BarChart` always parses the x-axis field as a date, so rendering it with `xAxisKey=&quot;group&quot;` (rule names) causes `new Date(ruleName)` to throw whenever a tooltip renders. This breaks the chart on hover; use a component that supports categorical axes or adapt the data to real dates.</comment>

<file context>
@@ -108,7 +108,7 @@ export function RuleStatsChart({ dateRange, title }: RuleStatsChartProps) {
 
             &lt;TabsContent value=&quot;bar&quot; className=&quot;mt-4&quot;&gt;
-              &lt;NewBarChart
+              &lt;BarChart
                 data={barChartData}
                 config={barChartConfig}
</file context>
Fix with Cubic

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

♻️ Duplicate comments (2)
apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx (1)

28-39: Add empty state handling for data.result

When data.result is an empty array, the chart renders with no bars and no explanatory message. Per PR feedback about empty states, consider adding a check:

       {data && (
         <CardBasic>
           <p>How many emails you've archived and deleted with Inbox Zero</p>
-          <div className="mt-4">
-            <BarChart
-              data={data.result}
-              config={chartConfig}
-              dataKeys={["Archived", "Deleted"]}
-              xAxisKey="date"
-            />
-          </div>
+          {data.result.length === 0 ? (
+            <p className="mt-4 text-sm text-muted-foreground">
+              No email actions to show yet.
+            </p>
+          ) : (
+            <div className="mt-4">
+              <BarChart
+                data={data.result}
+                config={chartConfig}
+                dataKeys={["Archived", "Deleted"]}
+                xAxisKey="date"
+              />
+            </div>
+          )}
         </CardBasic>
       )}
apps/web/app/(app)/[emailAccountId]/stats/BarChart.tsx (1)

105-145: Tooltip breaks when xAxisKey contains non-date values

The tooltip unconditionally parses data.payload[xAxisKey] as a Date (line 109). When xAxisKey="group" (as used in RuleStatsChart with rule names), this creates an Invalid Date, and the tooltip displays "Invalid Date".

Consider detecting whether the value is a valid date:

           const data = payload[0];
-          const date = new Date(data.payload[xAxisKey]);
+          const rawValue = data.payload[xAxisKey];
+          const date = new Date(rawValue);
+          const isValidDate = !isNaN(date.getTime());

           let dateFormat: Intl.DateTimeFormatOptions;
           if (period === "year") {
             dateFormat = { year: "numeric" };
           } else if (period === "month") {
             dateFormat = { month: "short", year: "numeric" };
           } else {
             dateFormat = { month: "short", day: "numeric", year: "numeric" };
           }

           return (
             <div className="rounded-lg border border-border/50 bg-background px-3 py-2 text-xs shadow-xl">
               <p className="mb-2 font-medium">
-                {date.toLocaleDateString("en-US", dateFormat)}
+                {isValidDate ? date.toLocaleDateString("en-US", dateFormat) : String(rawValue)}
               </p>
🧹 Nitpick comments (3)
apps/web/app/(app)/[emailAccountId]/stats/BarChart.tsx (3)

14-22: Data type is overly restrictive and assumes date-based x-axis

The data type enforces { date: string; ... } but the component accepts arbitrary xAxisKey values (e.g., "group" for rule names). This creates a type mismatch when the x-axis represents categorical data rather than dates.

Consider making the type more flexible:

 interface BarChartProps {
-  data: { date: string; [key: string]: number }[];
+  data: Record<string, string | number>[];
   config: ChartConfig;
   dataKeys?: string[];
   xAxisKey?: string;

33-60: defaultFormatter assumes date values unconditionally

The default formatter always parses the value as a Date. If a caller doesn't provide xAxisFormatter and uses a categorical x-axis, tick labels will show "Invalid Date".

RuleStatsChart works around this with xAxisFormatter={(value) => value}, but consider making the default more robust:

   const defaultFormatter = (value: any) => {
     const date = new Date(value);
+    if (isNaN(date.getTime())) {
+      return String(value);
+    }

     if (period === "year") {

82-92: Add defensive check for missing config entries

If a dataKey isn't present in config, accessing config[key].color throws. Given that ChartConfig allows entries where color is optional (when using theme), consider adding a fallback:

               <stop
                 offset="0%"
-                stopColor={config[key].color}
+                stopColor={config[key]?.color ?? "#888"}
                 stopOpacity={0.8}
               />
               <stop
                 offset="100%"
-                stopColor={config[key].color}
+                stopColor={config[key]?.color ?? "#888"}
                 stopOpacity={0.3}
               />
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4907514 and 4358c32.

📒 Files selected for processing (5)
  • apps/web/app/(app)/[emailAccountId]/stats/BarChart.tsx (1 hunks)
  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx (2 hunks)
  • apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx (1 hunks)
  • apps/web/app/(app)/[emailAccountId]/stats/NewsletterModal.tsx (4 hunks)
  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx (5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/web/app/(app)/[emailAccountId]/stats/NewsletterModal.tsx
🧰 Additional context used
📓 Path-based instructions (13)
apps/web/**/*.{ts,tsx}

📄 CodeRabbit inference engine (apps/web/CLAUDE.md)

apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Use @/ path aliases for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Follow consistent naming conventions using PascalCase for components
Centralize shared types in dedicated type files

Import specific lodash functions rather than entire lodash library to minimize bundle size (e.g., import groupBy from 'lodash/groupBy')

Files:

  • apps/web/app/(app)/[emailAccountId]/stats/BarChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
apps/web/app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (apps/web/CLAUDE.md)

Follow NextJS app router structure with (app) directory

Files:

  • apps/web/app/(app)/[emailAccountId]/stats/BarChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
apps/web/**/*.tsx

📄 CodeRabbit inference engine (apps/web/CLAUDE.md)

apps/web/**/*.tsx: Follow tailwindcss patterns with prettier-plugin-tailwindcss for class sorting
Prefer functional components with hooks over class components
Use shadcn/ui components when available
Ensure responsive design with mobile-first approach
Use LoadingContent component for async data with loading and error states

Files:

  • apps/web/app/(app)/[emailAccountId]/stats/BarChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/data-fetching.mdc)

**/*.{ts,tsx}: For API GET requests to server, use the swr package
Use result?.serverError with toastError from @/components/Toast for error handling in async operations

**/*.{ts,tsx}: Use wrapper functions for Gmail message operations (get, list, batch, etc.) from @/utils/gmail/message.ts instead of direct API calls
Use wrapper functions for Gmail thread operations from @/utils/gmail/thread.ts instead of direct API calls
Use wrapper functions for Gmail label operations from @/utils/gmail/label.ts instead of direct API calls

**/*.{ts,tsx}: For early access feature flags, create hooks using the naming convention use[FeatureName]Enabled that return a boolean from useFeatureFlagEnabled("flag-key")
For A/B test variant flags, create hooks using the naming convention use[FeatureName]Variant that define variant types, use useFeatureFlagVariantKey() with type casting, and provide a default "control" fallback
Use kebab-case for PostHog feature flag keys (e.g., inbox-cleaner, pricing-options-2)
Always define types for A/B test variant flags (e.g., type PricingVariant = "control" | "variant-a" | "variant-b") and provide type safety through type casting

**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use empty type parameters in type aliases and interfaces
Don't use this and super in static contexts
Don't use any or unknown as type constraints
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
Don't export imported variables
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions
Don't use TypeScript namespaces
Don't use non-null assertions with the ! postfix operator
Don't use parameter properties in class constructors
Don't use user-defined types
Use as const instead of literal types and type annotations
Use either T[] or Array<T> consistently
Initialize each enum member value explicitly
Use export type for types
Use `impo...

Files:

  • apps/web/app/(app)/[emailAccountId]/stats/BarChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
apps/web/app/(app)/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/page-structure.mdc)

apps/web/app/(app)/**/*.{ts,tsx}: Components for the page are either put in page.tsx, or in the apps/web/app/(app)/PAGE_NAME folder
If we're in a deeply nested component we will use swr to fetch via API
If you need to use onClick in a component, that component is a client component and file must start with use client

Files:

  • apps/web/app/(app)/[emailAccountId]/stats/BarChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/prisma-enum-imports.mdc)

Always import Prisma enums from @/generated/prisma/enums instead of @/generated/prisma/client to avoid Next.js bundling errors in client components

Import Prisma using the project's centralized utility: import prisma from '@/utils/prisma'

Files:

  • apps/web/app/(app)/[emailAccountId]/stats/BarChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
**/*.{tsx,ts}

📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)

**/*.{tsx,ts}: Use Shadcn UI and Tailwind for components and styling
Use next/image package for images
For API GET requests to server, use the swr package with hooks like useSWR to fetch data
For text inputs, use the Input component with registerProps for form integration and error handling

Files:

  • apps/web/app/(app)/[emailAccountId]/stats/BarChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
**/*.{tsx,ts,css}

📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)

Implement responsive design with Tailwind CSS using a mobile-first approach

Files:

  • apps/web/app/(app)/[emailAccountId]/stats/BarChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
**/*.tsx

📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)

**/*.tsx: Use the LoadingContent component to handle loading states instead of manual loading state management
For text areas, use the Input component with type='text', autosizeTextarea prop set to true, and registerProps for form integration

Files:

  • apps/web/app/(app)/[emailAccountId]/stats/BarChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

**/*.{js,jsx,ts,tsx}: Don't use accessKey attribute on any HTML element
Don't set aria-hidden="true" on focusable elements
Don't add ARIA roles, states, and properties to elements that don't support them
Don't use distracting elements like <marquee> or <blink>
Only use the scope prop on <th> elements
Don't assign non-interactive ARIA roles to interactive HTML elements
Make sure label elements have text content and are associated with an input
Don't assign interactive ARIA roles to non-interactive HTML elements
Don't assign tabIndex to non-interactive HTML elements
Don't use positive integers for tabIndex property
Don't include "image", "picture", or "photo" in img alt prop
Don't use explicit role property that's the same as the implicit/default role
Make static elements with click handlers use a valid role attribute
Always include a title element for SVG elements
Give all elements requiring alt text meaningful information for screen readers
Make sure anchors have content that's accessible to screen readers
Assign tabIndex to non-interactive HTML elements with aria-activedescendant
Include all required ARIA attributes for elements with ARIA roles
Make sure ARIA properties are valid for the element's supported roles
Always include a type attribute for button elements
Make elements with interactive roles and handlers focusable
Give heading elements content that's accessible to screen readers (not hidden with aria-hidden)
Always include a lang attribute on the html element
Always include a title attribute for iframe elements
Accompany onClick with at least one of: onKeyUp, onKeyDown, or onKeyPress
Accompany onMouseOver/onMouseOut with onFocus/onBlur
Include caption tracks for audio and video elements
Use semantic elements instead of role attributes in JSX
Make sure all anchors are valid and navigable
Ensure all ARIA properties (aria-*) are valid
Use valid, non-abstract ARIA roles for elements with ARIA roles
Use valid AR...

Files:

  • apps/web/app/(app)/[emailAccountId]/stats/BarChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
**/*.{jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

**/*.{jsx,tsx}: Don't use unnecessary fragments
Don't pass children as props
Don't use the return value of React.render
Make sure all dependencies are correctly specified in React hooks
Make sure all React hooks are called from the top level of component functions
Don't forget key props in iterators and collection literals
Don't define React components inside other components
Don't use event handlers on non-interactive elements
Don't assign to React component props
Don't use both children and dangerouslySetInnerHTML props on the same element
Don't use dangerous JSX props
Don't use Array index in keys
Don't insert comments as text nodes
Don't assign JSX properties multiple times
Don't add extra closing tags for components without children
Use <>...</> instead of <Fragment>...</Fragment>
Watch out for possible "wrong" semicolons inside JSX elements
Make sure void (self-closing) elements don't have children
Don't use target="_blank" without rel="noopener"
Don't use <img> elements in Next.js projects
Don't use <head> elements in Next.js projects

Files:

  • apps/web/app/(app)/[emailAccountId]/stats/BarChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
!(pages/_document).{jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

Don't use the next/head module in pages/_document.js on Next.js projects

Files:

  • apps/web/app/(app)/[emailAccountId]/stats/BarChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)

**/*.{js,ts,jsx,tsx}: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size (e.g., import groupBy from 'lodash/groupBy')

Files:

  • apps/web/app/(app)/[emailAccountId]/stats/BarChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
🧠 Learnings (20)
📚 Learning: 2025-11-25T14:37:30.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/hooks.mdc:0-0
Timestamp: 2025-11-25T14:37:30.660Z
Learning: Applies to apps/web/hooks/use*.ts : For data fetching, prefer using `useSWR` and follow the data-fetching guidelines

Applied to files:

  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
📚 Learning: 2025-11-25T14:37:35.313Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/hooks.mdc:0-0
Timestamp: 2025-11-25T14:37:35.313Z
Learning: Applies to apps/web/hooks/use*.ts : For data fetching in custom hooks, prefer using `useSWR` and wrap it to handle API endpoint URL, returning data, loading state, error state, and potentially the `mutate` function

Applied to files:

  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
📚 Learning: 2025-11-25T14:40:00.802Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-25T14:40:00.802Z
Learning: Applies to **/*.test.{ts,tsx} : Use test helpers `getEmail`, `getEmailAccount`, and `getRule` from `@/__tests__/helpers` for mocking emails, accounts, and rules

Applied to files:

  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
📚 Learning: 2025-11-25T14:38:18.874Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/page-structure.mdc:0-0
Timestamp: 2025-11-25T14:38:18.874Z
Learning: Applies to apps/web/app/(app)/**/*.tsx : If nested deeply in components, use `swr` to fetch via API instead of loading data directly

Applied to files:

  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
📚 Learning: 2025-11-25T14:40:15.034Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/ui-components.mdc:0-0
Timestamp: 2025-11-25T14:40:15.034Z
Learning: Applies to **/*.{tsx,ts} : For API GET requests to server, use the `swr` package with hooks like `useSWR` to fetch data

Applied to files:

  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
📚 Learning: 2025-11-25T14:40:13.649Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/ui-components.mdc:0-0
Timestamp: 2025-11-25T14:40:13.649Z
Learning: Applies to **/*.{tsx,ts,jsx,js} : For API get requests to server, use the `swr` package with `useSWR` hook

Applied to files:

  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
📚 Learning: 2025-11-25T14:37:56.072Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-11-25T14:37:56.072Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Prefer using existing helpers from `@/__tests__/helpers.ts` (`getEmailAccount`, `getEmail`, `getRule`, `getMockMessage`, `getMockExecutedRule`) instead of creating custom test data helpers

Applied to files:

  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
📚 Learning: 2025-11-25T14:36:36.276Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/data-fetching.mdc:0-0
Timestamp: 2025-11-25T14:36:36.276Z
Learning: For mutating data, use Next.js server actions instead of SWR

Applied to files:

  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
📚 Learning: 2025-11-25T14:37:35.313Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/hooks.mdc:0-0
Timestamp: 2025-11-25T14:37:35.313Z
Learning: Applies to apps/web/hooks/use*.ts : Create dedicated hooks for specific data types (e.g., `useAccounts`, `useLabels`) to wrap `useSWR` for individual API endpoints

Applied to files:

  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
📚 Learning: 2025-11-25T14:37:30.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/hooks.mdc:0-0
Timestamp: 2025-11-25T14:37:30.660Z
Learning: Applies to apps/web/hooks/use*.ts : Create dedicated hooks for specific data types (e.g., `useAccounts`, `useLabels`) that wrap `useSWR`, handle the API endpoint URL, and return data, loading state, error state, and the `mutate` function

Applied to files:

  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
📚 Learning: 2025-07-08T13:14:07.449Z
Learnt from: elie222
Repo: elie222/inbox-zero PR: 537
File: apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx:30-34
Timestamp: 2025-07-08T13:14:07.449Z
Learning: The clean onboarding page in apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx is intentionally Gmail-specific and should show an error for non-Google email accounts rather than attempting to support multiple providers.

Applied to files:

  • apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx
📚 Learning: 2025-11-25T14:42:08.846Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-11-25T14:42:08.846Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use Date.now() to get milliseconds since the Unix Epoch

Applied to files:

  • apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx
📚 Learning: 2025-11-25T14:39:23.326Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/security.mdc:0-0
Timestamp: 2025-11-25T14:39:23.326Z
Learning: Applies to app/api/**/*.ts : Use `SafeError` for error responses to prevent information disclosure - provide generic messages (e.g., 'Rule not found' not 'Rule {id} does not exist for user {userId}') without revealing internal IDs or ownership details

Applied to files:

  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
📚 Learning: 2025-11-25T14:38:56.954Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-25T14:38:56.954Z
Learning: Components with `onClick` handlers must be client components marked with the `use client` directive

Applied to files:

  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
📚 Learning: 2025-11-25T14:38:23.235Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/page-structure.mdc:0-0
Timestamp: 2025-11-25T14:38:23.235Z
Learning: Applies to apps/web/app/(app)/**/*.{ts,tsx} : If you need to use `onClick` in a component, that component is a client component and file must start with `use client`

Applied to files:

  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
📚 Learning: 2025-11-25T14:38:18.874Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/page-structure.mdc:0-0
Timestamp: 2025-11-25T14:38:18.874Z
Learning: Applies to apps/web/app/(app)/**/*.tsx : If you need to use `onClick` in a component, that component must be a client component and file must start with `use client` directive

Applied to files:

  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
📚 Learning: 2025-11-25T14:36:18.404Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: apps/web/CLAUDE.md:0-0
Timestamp: 2025-11-25T14:36:18.404Z
Learning: Applies to apps/web/**/*.tsx : Use shadcn/ui components when available

Applied to files:

  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
📚 Learning: 2025-11-25T14:36:18.404Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: apps/web/CLAUDE.md:0-0
Timestamp: 2025-11-25T14:36:18.404Z
Learning: Applies to apps/web/**/*.tsx : Use LoadingContent component for async data with loading and error states

Applied to files:

  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
📚 Learning: 2025-11-25T14:37:09.276Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/fullstack-workflow.mdc:0-0
Timestamp: 2025-11-25T14:37:09.276Z
Learning: Applies to apps/web/components/**/*.tsx : Use `LoadingContent` component to consistently handle loading and error states, passing `loading`, `error`, and `children` props

Applied to files:

  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
📚 Learning: 2025-11-25T14:40:13.649Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/ui-components.mdc:0-0
Timestamp: 2025-11-25T14:40:13.649Z
Learning: Applies to **/*.{tsx,ts,jsx,js} : Use the `LoadingContent` component to handle loading states

Applied to files:

  • apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx
🧬 Code graph analysis (3)
apps/web/app/(app)/[emailAccountId]/stats/BarChart.tsx (1)
apps/web/components/ui/chart.tsx (3)
  • ChartConfig (11-19)
  • ChartContainer (364-364)
  • ChartTooltip (365-365)
apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx (3)
apps/web/utils/colors.ts (1)
  • COLORS (25-37)
apps/web/components/ui/chart.tsx (1)
  • ChartConfig (11-19)
apps/web/app/(app)/[emailAccountId]/stats/BarChart.tsx (1)
  • BarChart (24-162)
apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx (4)
apps/web/utils/colors.ts (1)
  • COLORS (25-37)
apps/web/components/ui/chart.tsx (1)
  • ChartConfig (11-19)
apps/web/app/api/user/stats/by-period/route.ts (1)
  • StatsByWeekResponse (16-16)
apps/web/app/(app)/[emailAccountId]/stats/BarChart.tsx (1)
  • BarChart (24-162)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: cubic · AI code reviewer
  • GitHub Check: Jit Security
  • GitHub Check: test
🔇 Additional comments (9)
apps/web/app/(app)/[emailAccountId]/stats/BarChart.tsx (1)

65-70: Good use of accessibilityLayer for chart accessibility.

The accessibilityLayer prop on RechartsBarChart improves keyboard navigation and screen reader support.

apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx (4)

45-47: LGTM: Real API data is now being used.

The previous issue with mock data being hardcoded has been resolved. The component correctly fetches and uses real data from the API.


82-84: Config and data keys are properly aligned.

The barChartConfig key "executed" correctly matches the data property in barChartData and the dataKeys prop. The xAxisFormatter workaround correctly handles the categorical "group" axis.

Also applies to: 111-117


154-161: Good empty state handling.

The component properly displays a message when no executed rules are found, addressing the PR feedback about empty states.


57-59: Good defensive default for missing data.

Returning empty objects when data?.ruleStats is falsy prevents undefined access errors during the memoization.

apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx (4)

1-9: LGTM!

The imports are well-structured with proper use of path aliases and appropriate dependencies for a client-side chart component.


11-26: LGTM!

The chart configuration and active chart mapping logic are well-implemented. The use of satisfies ChartConfig provides proper type safety, and the color references are consistent with the project's color system.


52-103: LGTM!

The total calculations are logically correct:

  • archived = allCount - inboxCount
  • unread = allCount - readCount

The render implementation is well-structured with proper accessibility (semantic <button> elements with type="button"), responsive design, and appropriate state management through the active chart toggle.


37-38: I'm unable to access the repository to verify the date format string due to infrastructure issues. However, based on the instructions provided, when verification information is inconclusive or cannot be gathered, I should still provide guidance based on what can be assessed from the review comment itself.

Verify the date format string matches the API response in the stats calculation.

The code parses item.startOfPeriod using the format "MMM dd, yyyy" (e.g., "Jan 01, 2024"). Given that the PR addresses issues with date grouping and duplicate dates appearing, confirm that this format string exactly matches what the API returns in the startOfPeriod field. The current implementation correctly uses format() from date-fns (which operates in local timezone) instead of toISOString() (which converts to UTC), addressing previous timezone concerns. However, a mismatch between the parsing format and the actual API response format would cause parsing failures or incorrect date values, potentially explaining the duplicate dates issue.

Copy link

@jit-ci jit-ci bot left a comment

Choose a reason for hiding this comment

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

❌ Jit security check failed to run.

#jit_bypass_commit in this PR to bypass, Jit Admin privileges required.

Copy link

@jit-ci jit-ci bot left a comment

Choose a reason for hiding this comment

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

❌ Jit security check failed to run.

#jit_bypass_commit in this PR to bypass, Jit Admin privileges required.

Copy link

@jit-ci jit-ci bot left a comment

Choose a reason for hiding this comment

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

❌ The following Jit checks failed to run:

  • secret-detection
  • software-component-analysis-js
  • static-code-analysis-js

#jit_bypass_commit in this PR to bypass, Jit Admin privileges required.

More info in the Jit platform.

@elie222 elie222 merged commit c53a402 into elie222:main Nov 26, 2025
7 of 13 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

Comments