[WIP] Common Topics #1004
[WIP] Common Topics #1004
Conversation
This reverts commit ad4429f.
|
@jshwrnr is attempting to deploy a commit to the Inbox Zero OSS Program Team on Vercel. A member of the Team first needs to authorize it. |
|
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe pull request refactors the stats dashboard by replacing Tremor chart components with custom implementations (NewBarChart, BarListCard, MainStatChart, HorizontalBarChart), restructuring the layout with an ActionBar component, and removing obsolete dashboard components. Additionally, a comprehensive migration replaces Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant RulesSelect
participant Router
participant URL
User->>RulesSelect: Click DropdownMenu trigger
RulesSelect->>RulesSelect: Read current ruleId from searchParams
RulesSelect->>RulesSelect: Display getCurrentLabel()
User->>RulesSelect: Select rule from dropdown
RulesSelect->>RulesSelect: handleValueChange(newRuleId)
RulesSelect->>Router: router.push with updated ruleId param
Router->>URL: Update URL query params
RulesSelect->>RulesSelect: Re-render with new ruleId
sequenceDiagram
participant BarListCard
participant TabSelect
participant HorizontalBarChart
participant Dialog
BarListCard->>BarListCard: Initialize with tabs (Email, Domain, etc.)
Note over BarListCard: selectedTab = tabs[0]
BarListCard->>TabSelect: Render tab buttons
BarListCard->>HorizontalBarChart: Render main chart with selectedTab.data
rect rgba(200, 150, 255, 0.1)
Note over BarListCard,Dialog: User interaction: click item
HorizontalBarChart->>BarListCard: onItemClick(item)
BarListCard->>Dialog: Open with item details
Dialog->>HorizontalBarChart: Render larger chart in dialog
end
rect rgba(200, 200, 200, 0.1)
Note over TabSelect: User changes tab
TabSelect->>BarListCard: setSelectedTab(newTab)
BarListCard->>HorizontalBarChart: Update data from new tab
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (20)
apps/web/next.config.ts (1)
54-57: New image remotePattern fort1.gstatic.comis consistent; consider whether you want finer scopingAdding
t1.gstatic.comhere matches how the other hosts are configured and should let Next/Image handle those avatars correctly. Since CSP already includesimg-src ... https: ..., no extra CSP changes are required right now. If you later tighten CSP (e.g., remove the barehttps:source), remember to keept1.gstatic.comin sync there, and optionally consider adding apathnameconstraint if you want stricter scoping of allowed images from this host.apps/web/app/api/user/stats/by-period/route.ts (1)
58-58: Inconsistent alias naming.The
unreadalias is not quoted, while all other aliases use double quotes. For consistency and to avoid potential case-sensitivity issues, quote this alias like the others.- SUM(CASE WHEN read = false THEN 1 ELSE 0 END) AS unread, + SUM(CASE WHEN read = false THEN 1 ELSE 0 END) AS "unread",apps/web/components/StatsCard.tsx (1)
4-39: StatsCards implementation looks good; consider conditional subvalue renderingThe grid + Card composition is clean and data-driven. One small improvement: avoid rendering an empty
<span>whensubvalueis undefined.- <div className=""> - <span className="text-2xl font-bold">{stat.value}</span> - <span className="ml-2 text-sm text-muted-foreground"> - {stat.subvalue} - </span> - </div> + <div> + <span className="text-2xl font-bold">{stat.value}</span> + {stat.subvalue && ( + <span className="ml-2 text-sm text-muted-foreground"> + {stat.subvalue} + </span> + )} + </div>apps/web/components/ui/progress.tsx (1)
9-23: Progress innerClassName extension is reasonable; confirm default color and import pathThe
innerClassNameprop is a nice way to customize the indicator bar without forking the component. Two things to double‑check:
- The default
"bg-blue-500"changes behavior if you previously relied on a theme token likebg-primary. Make sure this is intentional for the new design system.- Other files import
cnfrom"@/utils"while this one uses"@/utils/index". Aligning on a single path would avoid accidental duplication.apps/web/components/ui/calendar.tsx (1)
28-29: Consider using theme-aware border colors.The border color
border-gray-200is hardcoded in multiple places. This may not adapt well to dark mode or custom themes.Consider using a theme-aware token:
- caption_start: "p-3", - caption_end: "p-3 border-l border-gray-200", + caption_start: "p-3", + caption_end: "p-3 border-l border-border",- <div className="p-3 border-l border-gray-200">{rightContent}</div> + <div className="p-3 border-l border-border">{rightContent}</div>Also applies to: 68-68
apps/web/components/DatePickerWithRange.tsx (2)
44-44: useMemo for current date may not provide intended stability.Using
useMemo(() => new Date(), [])creates a "now" timestamp that remains constant for the component's lifetime, which means the "Last day" selection won't update to the actual current day if the component stays mounted across midnight.Consider if this behavior is intentional. If you want "now" to always reflect the actual current time, remove the memoization:
- const now = useMemo(() => new Date(), []); + const now = new Date();Alternatively, if you want to cache it but update periodically, add a refresh mechanism.
100-100: Add radix parameter to parseInt for clarity.The
Number.parseInt(value)call should include the radix parameter (base 10) for explicit clarity and to avoid potential parsing issues.- from: subDays(now, Number.parseInt(value)), + from: subDays(now, Number.parseInt(value, 10)),apps/web/components/charts/DomainIcon.tsx (1)
55-75: Implement favicon caching and request deduplication to reduce dependency on Google's undocumented favicon service.Verification confirms the concerns are valid:
- The t1.gstatic.com/faviconV2 endpoint is undocumented and Google does not publish rate limits or formal SLAs
- Current code (line 5-10:
apps/web/components/charts/DomainIcon.tsx) constructs the favicon URL on every call without caching- DomainIcon is used in loops (HorizontalBarChart.tsx, line 50), generating multiple independent API requests
- Existing fallback (FallbackIcon) prevents failures but does not prevent repeated or throttled requests
Recommended improvements:
- Implement domain-level favicon caching (e.g., at module level or via React Query/SWR to deduplicate requests)
- Add request timeout/retry logic to gracefully handle service throttling
- Consider a favicon CDN alternative or self-hosted fallback service for critical deployments
apps/web/components/ui/button.tsx (1)
34-34: Consider a more semantic size variant name.The
"xs-2"naming is unconventional. Consider alternatives like"xs-md","2xs", or a more descriptive name that clearly indicates it sits between"xs"and"sm".Example refactor:
- "xs-2": "h-7 rounded-md px-2 text-xs", + "2xs": "h-7 rounded-md px-2 text-xs",Or:
- "xs-2": "h-7 rounded-md px-2 text-xs", + "xs-md": "h-7 rounded-md px-2 text-xs",apps/web/app/(app)/[emailAccountId]/stats/BarListCard.tsx (2)
18-18: Address the TODO for type safety.The
dataproperty in thetabsarray is typed asany. Consider defining a proper type for the data structure to improve type safety.Do you want me to help define the type based on how
HorizontalBarChartexpects the data to be structured?
32-34: Memoize selected tab lookup for better performance.The expression
tabs.find((d) => d.id === selected)is repeated multiple times (lines 59, 64, 83). Consider memoizing this lookup to avoid recalculating on each render.Apply this refactor:
+import { TabSelect } from "@/components/TabSelect"; +import { Card, CardContent, CardHeader } from "@/components/ui/card"; +import { HorizontalBarChart } from "@/components/charts/HorizontalBarChart"; -import { useState } from "react"; +import { useState, useMemo } from "react"; import { Button } from "@/components/ui/button"; // ... rest of imports export function BarListCard({ tabs, icon, title, onItemClick, hideIcons, }: BarListCardProps) { const [selected, setSelected] = useState<string | null>( tabs?.length > 0 ? tabs[0]?.id : null, ); + const selectedTab = useMemo( + () => tabs.find((d) => d.id === selected), + [tabs, selected] + ); + return ( <Card className="h-full bg-background relative"> {/* ... */} <HorizontalBarChart - data={tabs.find((d) => d.id === selected)?.data || []} + data={selectedTab?.data || []} onItemClick={onItemClick} hideIcon={hideIcons} /> <div className="absolute w-full left-0 bottom-0 pb-6 z-30"> - {tabs.find((d) => d.id === selected)?.data.length > 0 && ( + {(selectedTab?.data.length ?? 0) > 0 && ( <div className="flex justify-center"> <Dialog> {/* ... */} <HorizontalBarChart - data={tabs.find((d) => d.id === selected)?.data || []} + data={selectedTab?.data || []} onItemClick={onItemClick} hideIcon={hideIcons} /> </Dialog> </div> )} </div> </Card> ); }Also applies to: 59-59, 64-64, 83-83
apps/web/app/(app)/[emailAccountId]/bulk-unsubscribe/BulkUnsubscribeSection.tsx (1)
250-276: Simplify nested div structure.The Toggle component is wrapped in unnecessary nested divs (lines 251-253). The outer div has flex styling, but the inner divs add no semantic or styling value.
Apply this diff to simplify:
- <ActionBar rightContent={<LoadStatsButton />}> - <div className="flex items-center justify-end gap-1"> - <div className=""> - <Toggle - name="show-unhandled" - label="Only unhandled" - enabled={onlyUnhandled} - onChange={() => - setFilters( - onlyUnhandled - ? { - unhandled: true, - autoArchived: true, - unsubscribed: true, - approved: true, - } - : { - unhandled: true, - autoArchived: false, - unsubscribed: false, - approved: false, - }, - ) - } - /> - </div> - </div> + <ActionBar rightContent={<LoadStatsButton />}> + <Toggle + name="show-unhandled" + label="Only unhandled" + enabled={onlyUnhandled} + onChange={() => + setFilters( + onlyUnhandled + ? { + unhandled: true, + autoArchived: true, + unsubscribed: true, + approved: true, + } + : { + unhandled: true, + autoArchived: false, + unsubscribed: false, + approved: false, + }, + ) + } + /> <SearchBar onSearch={setSearch} />apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx (1)
8-15: NewBarChart wiring and color config look solid; consider sharing types with the API responseThe
chartConfigkeys anddataKeys={["Archived", "Deleted"]}line up, and usingCOLORS.analyticskeeps things consistent across charts. The only thing to watch is that/api/user/stats/email-actionscontinues to returndate,Archived, andDeletedfields with those exact names; if the backend ever changes casing or naming, this chart will silently break. If you have a shared type for the response shape, you could reuse it here (or derivedataKeysfrom that type) to catch such changes at compile time.Also applies to: 30-38
apps/web/components/Banner.tsx (1)
3-20: Banner API change is clear; double‑check all callsites and children usageThe new
{ title, children }shape and UnicornScene background look good. SinceonCloseanddescriptionwere removed andtitleis nowReact.ReactNode, just make sure all existing usages have been updated (especially anything relying on a dismissible banner). Also, thetypeof children === "string"branch is fine for simple messages, but if callers might pass numbers or arrays you may want to broaden that condition or always wrap in a<p>for consistent spacing.apps/web/utils/colors.ts (1)
25-37: COLORS token map is straightforward; consideras constfor stronger typingThe COLORS structure is clear and works well with the chart config usage. To get stricter typing (and prevent accidental mutation), you could append
as constto the object so consumers see literal keys and values:export const COLORS = { analytics: { blue: "#006EFF80", purple: "#6410FF80", pink: "#C942B2", lightPink: "#C942B260", green: "#17A34A", lightGreen: "#17A34A60", }, footer: { gray: "#4E4E4E", }, } as const;apps/web/app/(app)/[emailAccountId]/stats/TopicDistribution.tsx (2)
16-79: Mock topic + daily data generation is fine; consider stabilizing per topicThe mock topics and
generateDailyDatafunction are clear and produce reasonable demo data. BecausegenerateDailyData(selectedTopic.count)is called directly in the JSX, every re‑render of the dialog regenerates random values, which can cause the chart to jump even when nothing “changed” from the user’s perspective (and may run twice in React strict mode). If you want the daily series to stay stable per topic while the dialog is open, you could memoize by topic name or precomputedailyDataas part ofMOCK_TOPICS.
81-151: Dialog open handling works but could use theopenargument for clarityUsing
open={!!selectedTopic}andonOpenChange={() => setSelectedTopic(null)}correctly closes the dialog whenever the user dismisses it. For a slightly clearer contract with the Dialog API, you might capture theopenvalue and only clear the selection when it becomes false:<Dialog open={!!selectedTopic} onOpenChange={(open) => { if (!open) setSelectedTopic(null); }} >This makes it obvious that you’re reacting to the dialog closing, and gives you flexibility if you ever add a trigger that toggles it open.
apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx (1)
11-103: MainStatChart data mapping looks correct; only minor polish opportunitiesThe chartConfig,
chartDatamapping, andtotalderivations all line up with the expected field names (All,Read,Sent,Archived,Unread,Unarchived/inboxCount), and the activeChart logic cleanly switches between the different views while still showing paired series for read/unread and archived/inbox. A few small polish ideas:
getActiveChart(activChart: ...)has a minor typo in the parameter name; renaming toactiveChartwould improve readability.- If the API ever changes the
startOfPeriodformat away from"MMM dd, yyyy", theparsecall will start producingInvalid Datesilently. If there’s a shared type or helper for this date field elsewhere, reusing it here would help keep things in sync.- Optionally, you could derive the header button list from
chartConfig(or a small config array) instead of hard‑coding["received", "sent", "read", "archived"]to reduce duplication if new metrics are added later.Functionally, though, this component looks good as‑is.
apps/web/app/(app)/[emailAccountId]/stats/Stats.tsx (1)
15-21: Stats header and period/date wiring look correct; minor layout nit
- Title derivation and
PageHeadingusage are clear and avoid duplication.DatePickerWithRange+onSetDateDropdownlogic keepsperiodreasonable for shorter ranges and is side‑effect safe.StatsSummary,EmailAnalytics,TopicDistribution, andRuleStatsChartall share the samedateRange/refreshInterval/periodas expected.One minor UX nit: the wrapper around
TopicDistributionusesgrid grid-cols-2but only contains a single child, so half the width is unused. If that isn’t intentional, considergrid-cols-1 md:grid-cols-2or dropping the grid until a second card is added.Also applies to: 73-77, 80-133
apps/web/components/charts/HorizontalBarChart.tsx (1)
81-92: Key uniqueness assumption.The component uses
item.nameas the React key (lines 84, 96), which assumes that names are unique within the data array. If duplicate names could exist, consider using an index or a more unique identifier.If duplicates are possible, apply this diff:
-{data.map((item) => { +{data.map((item, index) => { // ... content ... if (onItemClick) { return ( <button - key={item.name} + key={`${item.name}-${index}`} type="button" // ...Also applies to: 94-101
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (55)
apps/web/app/(app)/[emailAccountId]/assistant/RulesSelect.tsx(1 hunks)apps/web/app/(app)/[emailAccountId]/bulk-unsubscribe/BulkUnsubscribeDesktop.tsx(2 hunks)apps/web/app/(app)/[emailAccountId]/bulk-unsubscribe/BulkUnsubscribeSection.tsx(3 hunks)apps/web/app/(app)/[emailAccountId]/bulk-unsubscribe/BulkUnsubscribeSummary.tsx(0 hunks)apps/web/app/(app)/[emailAccountId]/stats/ActionBar.tsx(1 hunks)apps/web/app/(app)/[emailAccountId]/stats/BarListCard.tsx(1 hunks)apps/web/app/(app)/[emailAccountId]/stats/CombinedStatsChart.tsx(0 hunks)apps/web/app/(app)/[emailAccountId]/stats/DetailedStats.tsx(0 hunks)apps/web/app/(app)/[emailAccountId]/stats/DetailedStatsFilter.tsx(2 hunks)apps/web/app/(app)/[emailAccountId]/stats/EmailActionsAnalytics.tsx(2 hunks)apps/web/app/(app)/[emailAccountId]/stats/EmailAnalytics.tsx(2 hunks)apps/web/app/(app)/[emailAccountId]/stats/LoadStatsButton.tsx(2 hunks)apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx(1 hunks)apps/web/app/(app)/[emailAccountId]/stats/NewBarChart.tsx(1 hunks)apps/web/app/(app)/[emailAccountId]/stats/NewsletterModal.tsx(4 hunks)apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx(5 hunks)apps/web/app/(app)/[emailAccountId]/stats/Stats.tsx(2 hunks)apps/web/app/(app)/[emailAccountId]/stats/StatsChart.tsx(0 hunks)apps/web/app/(app)/[emailAccountId]/stats/StatsSummary.tsx(1 hunks)apps/web/app/(app)/[emailAccountId]/stats/TopicDistribution.tsx(1 hunks)apps/web/app/(marketing)(1 hunks)apps/web/app/api/user/stats/by-period/route.ts(1 hunks)apps/web/components/Banner.tsx(1 hunks)apps/web/components/DatePickerWithRange.tsx(2 hunks)apps/web/components/List.tsx(1 hunks)apps/web/components/PageHeader.tsx(1 hunks)apps/web/components/ProgressPanel.tsx(2 hunks)apps/web/components/StatsCard.tsx(1 hunks)apps/web/components/TabSelect.tsx(2 hunks)apps/web/components/charts/BarList.tsx(0 hunks)apps/web/components/charts/DomainIcon.tsx(1 hunks)apps/web/components/charts/HorizontalBarChart.tsx(1 hunks)apps/web/components/common/Banner.tsx(0 hunks)apps/web/components/new-landing/BrandScroller.tsx(3 hunks)apps/web/components/new-landing/CallToAction.tsx(2 hunks)apps/web/components/new-landing/FooterLineLogo.tsx(2 hunks)apps/web/components/new-landing/HeaderLinks.tsx(1 hunks)apps/web/components/new-landing/UnicornScene.tsx(2 hunks)apps/web/components/new-landing/common/Anchor.tsx(2 hunks)apps/web/components/new-landing/common/Badge.tsx(2 hunks)apps/web/components/new-landing/common/BlurFade.tsx(2 hunks)apps/web/components/new-landing/common/Button.tsx(2 hunks)apps/web/components/new-landing/common/Card.tsx(2 hunks)apps/web/components/new-landing/common/DisplayCard.tsx(2 hunks)apps/web/components/new-landing/common/Section.tsx(3 hunks)apps/web/components/new-landing/common/Typography.tsx(5 hunks)apps/web/components/new-landing/common/WordReveal.tsx(2 hunks)apps/web/components/new-landing/sections/Pricing.tsx(2 hunks)apps/web/components/ui/button.tsx(1 hunks)apps/web/components/ui/calendar.tsx(2 hunks)apps/web/components/ui/progress.tsx(2 hunks)apps/web/next.config.ts(1 hunks)apps/web/package.json(0 hunks)apps/web/tailwind.config.js(1 hunks)apps/web/utils/colors.ts(1 hunks)
💤 Files with no reviewable changes (7)
- apps/web/app/(app)/[emailAccountId]/stats/CombinedStatsChart.tsx
- apps/web/app/(app)/[emailAccountId]/bulk-unsubscribe/BulkUnsubscribeSummary.tsx
- apps/web/app/(app)/[emailAccountId]/stats/StatsChart.tsx
- apps/web/components/common/Banner.tsx
- apps/web/app/(app)/[emailAccountId]/stats/DetailedStats.tsx
- apps/web/package.json
- apps/web/components/charts/BarList.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-08-23T11:37:26.779Z
Learnt from: aryanprince
Repo: elie222/inbox-zero PR: 210
File: apps/web/app/(app)/stats/NewsletterModal.tsx:2-4
Timestamp: 2024-08-23T11:37:26.779Z
Learning: `MoreDropdown` is a React component and `useUnsubscribeButton` is a custom React hook, and they should not be imported using `import type`.
Applied to files:
apps/web/app/(app)/[emailAccountId]/bulk-unsubscribe/BulkUnsubscribeDesktop.tsxapps/web/app/(app)/[emailAccountId]/stats/NewsletterModal.tsxapps/web/app/(app)/[emailAccountId]/assistant/RulesSelect.tsxapps/web/app/(app)/[emailAccountId]/stats/DetailedStatsFilter.tsxapps/web/components/new-landing/sections/Pricing.tsxapps/web/components/DatePickerWithRange.tsx
📚 Learning: 2025-07-17T04:19:57.099Z
Learnt from: edulelis
Repo: elie222/inbox-zero PR: 576
File: packages/resend/emails/digest.tsx:78-83
Timestamp: 2025-07-17T04:19:57.099Z
Learning: In packages/resend/emails/digest.tsx, the DigestEmailProps type uses `[key: string]: DigestItem[] | undefined | string | Date | undefined` instead of intersection types like `& Record<string, DigestItem[] | undefined>` due to implementation constraints. This was the initial implementation approach and cannot be changed to more restrictive typing.
Applied to files:
apps/web/app/(app)/[emailAccountId]/stats/NewsletterModal.tsxapps/web/app/(app)/[emailAccountId]/stats/EmailAnalytics.tsx
📚 Learning: 2025-06-05T09:49:12.168Z
Learnt from: elie222
Repo: elie222/inbox-zero PR: 485
File: apps/web/app/(landing)/login/page.tsx:41-43
Timestamp: 2025-06-05T09:49:12.168Z
Learning: In Next.js App Router, components that use the `useSearchParams` hook require a Suspense boundary to handle the asynchronous nature of search parameter access. The Suspense wrapper is necessary and should not be removed when a component uses useSearchParams.
Applied to files:
apps/web/app/(app)/[emailAccountId]/assistant/RulesSelect.tsx
🧬 Code graph analysis (15)
apps/web/app/(app)/[emailAccountId]/stats/TopicDistribution.tsx (3)
apps/web/utils/colors.ts (1)
COLORS(25-37)apps/web/app/(app)/[emailAccountId]/stats/BarListCard.tsx (1)
BarListCard(25-96)apps/web/app/(app)/[emailAccountId]/stats/NewBarChart.tsx (1)
NewBarChart(18-152)
apps/web/app/(app)/[emailAccountId]/stats/BarListCard.tsx (4)
apps/web/components/new-landing/common/Card.tsx (3)
Card(67-105)CardHeader(22-54)CardContent(10-12)apps/web/components/TabSelect.tsx (1)
TabSelect(44-101)apps/web/components/charts/HorizontalBarChart.tsx (1)
HorizontalBarChart(21-105)apps/web/components/new-landing/common/Button.tsx (1)
Button(16-100)
apps/web/components/new-landing/FooterLineLogo.tsx (1)
apps/web/utils/colors.ts (1)
COLORS(25-37)
apps/web/app/(app)/[emailAccountId]/bulk-unsubscribe/BulkUnsubscribeDesktop.tsx (2)
apps/web/components/Tooltip.tsx (1)
Tooltip(18-41)apps/web/components/ui/progress.tsx (1)
Progress(29-29)
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/NewBarChart.tsx (1)
NewBarChart(18-152)
apps/web/app/(app)/[emailAccountId]/stats/StatsSummary.tsx (3)
apps/web/app/api/user/stats/by-period/route.ts (2)
StatsByWeekParams(15-15)StatsByWeekResponse(16-16)apps/web/hooks/useOrgSWR.ts (1)
useOrgSWR(10-45)apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx (1)
MainStatChart(28-104)
apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx (5)
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/components/new-landing/common/Card.tsx (2)
Card(67-105)CardContent(10-12)apps/web/app/(app)/[emailAccountId]/stats/NewBarChart.tsx (1)
NewBarChart(18-152)
apps/web/components/StatsCard.tsx (1)
apps/web/components/new-landing/common/Card.tsx (3)
Card(67-105)CardHeader(22-54)CardContent(10-12)
apps/web/app/(app)/[emailAccountId]/stats/NewsletterModal.tsx (2)
apps/web/app/(app)/[emailAccountId]/stats/NewBarChart.tsx (1)
NewBarChart(18-152)apps/web/utils/colors.ts (1)
COLORS(25-37)
apps/web/components/charts/HorizontalBarChart.tsx (1)
apps/web/components/charts/DomainIcon.tsx (1)
DomainIcon(55-75)
apps/web/components/ProgressPanel.tsx (1)
apps/web/components/ui/progress.tsx (1)
Progress(29-29)
apps/web/components/Banner.tsx (1)
apps/web/components/new-landing/UnicornScene.tsx (1)
UnicornScene(26-55)
apps/web/app/(app)/[emailAccountId]/bulk-unsubscribe/BulkUnsubscribeSection.tsx (3)
apps/web/app/(app)/[emailAccountId]/stats/ActionBar.tsx (1)
ActionBar(9-25)apps/web/app/(app)/[emailAccountId]/stats/LoadStatsButton.tsx (1)
LoadStatsButton(8-27)apps/web/components/DatePickerWithRange.tsx (1)
DatePickerWithRange(37-110)
apps/web/app/(app)/[emailAccountId]/stats/Stats.tsx (6)
apps/web/components/PageWrapper.tsx (1)
PageWrapper(3-20)apps/web/app/(app)/[emailAccountId]/stats/ActionBar.tsx (1)
ActionBar(9-25)apps/web/app/(app)/[emailAccountId]/stats/LoadStatsButton.tsx (1)
LoadStatsButton(8-27)apps/web/app/(app)/[emailAccountId]/stats/DetailedStatsFilter.tsx (1)
DetailedStatsFilter(18-79)apps/web/app/(app)/[emailAccountId]/stats/StatsSummary.tsx (1)
StatsSummary(14-49)apps/web/app/(app)/[emailAccountId]/stats/TopicDistribution.tsx (1)
TopicDistribution(81-154)
apps/web/components/DatePickerWithRange.tsx (1)
apps/web/components/List.tsx (1)
List(16-39)
⏰ 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: Review for correctness
🔇 Additional comments (32)
apps/web/app/(marketing) (1)
1-1: Verify the submodule reference update is intentional and document its purpose.This commit updates the submodule pointer. Since the actual code changes occur within the submodule (outside this view), verify that:
- The new commit hash (
e3c81eb5e0d7950b89d45a8c3a57e568d3072028) contains the intended changes.- The submodule update aligns with the PR's refactoring goals (dashboard components, Tremor removal, etc.).
- No breaking changes or new dependencies are introduced in the submodule.
Please confirm the submodule changes are intentional and document what functional updates are included in the new commit reference.
apps/web/components/new-landing/sections/Pricing.tsx (1)
35-35: LGTM! Clean refactoring fromcxtocnutility.The migration from
class-variance-authority'scxto the localcnutility is correctly applied—both the import and usage are updated consistently. The conditional class merging logic remains unchanged.Also applies to: 114-117
apps/web/components/PageHeader.tsx (1)
14-20: LGTM! Clean refactor to extract prop types.The interface is well-defined and the function signature correctly uses it. This improves code organization by extracting inline types into a named interface.
apps/web/components/new-landing/common/Anchor.tsx (1)
1-1: LGTM! Clean migration to cn utility.The replacement of
cxwithcnis consistent with the broader refactor across the codebase.Also applies to: 16-16
apps/web/components/new-landing/common/Section.tsx (1)
1-1: LGTM! Consistent utility migration.The migration from
cxtocnis applied correctly across both components.Also applies to: 11-11, 29-29
apps/web/components/new-landing/common/Typography.tsx (1)
1-2: LGTM! Proper separation of utilities.The migration correctly replaces
cxwithcnfor class merging while appropriately retainingcvafor variant styles in the Paragraph component.Also applies to: 12-15, 37-39, 49-51, 63-66
apps/web/components/new-landing/BrandScroller.tsx (1)
8-8: LGTM! Consistent migration.The replacement of
cxwithcnis applied correctly throughout the component.Also applies to: 26-29, 39-39
apps/web/components/new-landing/common/BlurFade.tsx (1)
11-11: LGTM! Clean utility replacement.The migration from
cxtocnis correctly applied.Also applies to: 66-66
apps/web/components/TabSelect.tsx (1)
32-32: Verify the visual impact of removing horizontal padding.The removal of
px-1.5from the indicator will change its width. Ensure this visual change is intentional and aligns with the design.apps/web/components/new-landing/CallToAction.tsx (1)
8-8: LGTM! Consistent with the migration pattern.The replacement of
cxwithcnis correctly applied.Also applies to: 24-24
apps/web/components/ProgressPanel.tsx (1)
6-6: ✓ Verified: Progress component supportsinnerClassNameprop for dynamic color controlThe Progress component in
apps/web/components/ui/progress.tsxproperly implements theinnerClassNameprop as an optional string parameter (defaulting to"bg-blue-500"), which is correctly applied to the progress indicator element. The implementation supports dynamic color changes based on completion state as intended.apps/web/components/new-landing/common/DisplayCard.tsx (1)
2-36: cn migration in DisplayCard looks correct
cnimport and bothclassName={cn(..., className)}usages preserve the previous behavior and keep thecenterContentconditional intact. No issues from this refactor.apps/web/components/new-landing/common/Card.tsx (1)
2-43: Consistent cn usage in Card helpersSwitching to
cnforCardContentand theCardHeadertitle preserves existing styles while aligning with the repo-wide utility. No functional concerns.apps/web/components/new-landing/HeaderLinks.tsx (1)
35-88: Semantic color tokens and EnhancedListItem wiring look consistentAll
useCasesentries provide the expectediconColor,borderColor,gradient, andhoverBgvalues, andEnhancedListItemcleanly threads them intocn(...)calls. This should make future theming straightforward with no behavioral regressions.Also applies to: 169-188
apps/web/app/(app)/[emailAccountId]/bulk-unsubscribe/BulkUnsubscribeDesktop.tsx (1)
18-23: Progress + tooltip refactor is sound; verify data invariantsReplacing Tremor with
<Progress>and Radix tooltips retains the xl/2xl breakpoint behavior and adds clearer percentage labels. Tooltip messages correctly derive:
- read:
readEmailsvsvalue - readEmails- archived:
archivedEmailsvsitem.inboxEmailsJust ensure upstream data keeps
readEmails <= valueandarchivedEmails + inboxEmailsin sync to avoid confusing counts.Also applies to: 124-154
apps/web/app/(app)/[emailAccountId]/assistant/RulesSelect.tsx (1)
4-66: Dropdown-based rule selection + URL sync looks correct; ensure Suspense wrapper upstreamThe new DropdownMenu flow correctly:
- Reads
ruleIdfromuseSearchParamswith"all"fallback.- Updates just the
ruleIdquery parameter while preserving others viaURLSearchParams.- Derives the trigger label from the current value, gracefully falling back to
"All rules"if a rule id is missing.- Narrows the loading skeleton to match the trigger width.
Since this component now uses
useSearchParams, please confirm that its parent tree still wraps it in<Suspense>to avoid App Router warnings or hydration issues. Based on learnings, components usinguseSearchParamsin this repo are expected to sit under a Suspense boundary.apps/web/components/new-landing/UnicornScene.tsx (1)
3-3: LGTM! Clean migration from cx to cn utility.The replacement of
cxwith the localcnutility is consistent with the broader refactoring effort across the codebase. No behavioral changes, and the implementation remains correct.Also applies to: 52-52
apps/web/app/(app)/[emailAccountId]/stats/DetailedStatsFilter.tsx (1)
14-14: LGTM! Improved dropdown affordance.The addition of the ChevronDown icon and the change from "ghost" to "outline" variant improve the visual clarity that this is a dropdown button. Good UX enhancement.
Also applies to: 46-46, 52-52
apps/web/components/new-landing/common/Button.tsx (1)
1-3: LGTM! Correct migration from cx to cn.The replacement of
cxwithcnwhile keepingcvais correct. Thecvalibrary is still needed for variant composition, whilecnhandles the className merging. This aligns with the broader refactoring pattern.Also applies to: 63-63
apps/web/components/new-landing/common/WordReveal.tsx (1)
2-2: LGTM! Consistent with cx to cn migration.Clean replacement of the
cxutility with the localcnutility. No behavioral changes and implementation remains correct.Also applies to: 32-32
apps/web/tailwind.config.js (1)
86-176: Color tokens are actively used throughout the codebase.Verification confirms the sidebar colors and new color palette are properly integrated:
- Sidebar colors are extensively used in
apps/web/components/ui/sidebar.tsxand related components via Tailwind classes (bg-sidebar,text-sidebar-foreground,bg-sidebar-accent, etc.)- New color palette is actively used in
apps/web/components/new-landing/HeaderLinks.tsxandapps/web/components/new-landing/common/Badge.tsxfor landing page styling across all color families (purple, green, blue, indigo, pink, orange, yellow, brown, red, cyan, gray)No bloat detected.
apps/web/components/ui/calendar.tsx (1)
23-23: I've located the Calendar component usages. There are two places where the Calendar UI component is used:
- apps/web/components/DatePickerWithRange.tsx (line 82) — using
mode="range"- apps/web/app/(app)/[emailAccountId]/assistant/SetDateDropdown.tsx (line 45) — using
mode="single"Both usages are inside
PopoverContentcomponents withclassName="w-auto p-0", meaning the Calendar is responsible for its own internal spacing.Looking at the updated calendar.tsx, the padding is now applied via the
classNamesobject:
caption_start: "p-3"andcaption_end: "p-3 border-l border-gray-200"provide padding to the caption (month header) elements- The day grid itself has spacing defined through
space-y-1and cell dimensions (h-9 w-9)The structural change appears sound—padding is moved from an outer wrapper to specific caption elements. However, this requires manual verification since the concern about visual coverage cannot be definitively resolved without seeing actual rendering or visual regression tests. The caption padding applies to the month header row, but the complete visual appearance of the calendar (including spacing around the entire day grid) depends on whether the classNames provide sufficient spacing when there's no outer padding.
Please verify that both Calendar usages render with appropriate spacing and no visual regressions in the date picker and date dropdown components.
apps/web/app/(app)/[emailAccountId]/stats/LoadStatsButton.tsx (1)
3-3: LGTM!The icon change from
AreaChartIcontoRefreshCcwand the updated button labels ("Loading more..." / "Load more") better convey the button's purpose of loading additional data.Also applies to: 21-23
apps/web/components/new-landing/FooterLineLogo.tsx (1)
1-1: LGTM!Migrating from the hardcoded hex value to
COLORS.footer.grayaligns with the project-wide color system refactoring. This improves maintainability by centralizing color definitions.Also applies to: 20-20
apps/web/components/new-landing/common/Badge.tsx (1)
35-76: LGTM!The migration from explicit hex colors to token-based color classes (e.g.,
text-new-blue-600,from-new-blue-150) is consistent across all badge variants. This aligns with the project-wide color system refactoring and improves maintainability.apps/web/app/(app)/[emailAccountId]/stats/NewsletterModal.tsx (2)
51-51: Verify dialog content scrolling behavior.The
overflow-scrollclass was removed fromDialogContent. Ensure that content overflow is handled appropriately, especially when displaying large datasets in the modal.
154-160: Data structure verified—migration is correct.The
getSenderEmailsfunction returns{ result: Array<{ startOfPeriod: string; Emails: number }> }, which matches theNewBarChartexpectations perfectly. ThestartOfPeriodkey is used forxAxisKeyandEmailsis configured in the chart config. No issues found.apps/web/app/(app)/[emailAccountId]/bulk-unsubscribe/BulkUnsubscribeSection.tsx (1)
250-341: LGTM!The integration with
ActionBarandDatePickerWithRangefollows the new consolidated header pattern correctly. TheLoadStatsButtonis appropriately placed in therightContentslot, and all necessary props are passed toDatePickerWithRange.apps/web/app/(app)/[emailAccountId]/stats/ActionBar.tsx (1)
3-23: Simplified ActionBar API and layout look solidProps and flex layout are straightforward and match how
ActionBaris used elsewhere (header controls + right-aligned actions). No functional concerns here.apps/web/app/(app)/[emailAccountId]/stats/RuleStatsChart.tsx (1)
25-27: NewBarChart integration and configs are consistent
barChartData(group+executed) matchesbarChartConfiganddataKeys={["executed"]}.- Using
xAxisKey="group"withxAxisFormatter={(value) => value}correctly bypasses the date formatter.- Pie chart config and “no data” state in
CardBasicare wired sensibly.No issues from a data/typing standpoint.
Also applies to: 53-55, 57-60, 82-90, 100-117, 149-152, 155-160
apps/web/app/(app)/[emailAccountId]/stats/StatsSummary.tsx (1)
12-38: Clean period threading into API and MainStatChart
- Adding
periodtoStatsSummaryprops and including it inStatsByWeekParamsaligns with the new chart behavior.- The
Object.entries+toString+URLSearchParamsconstruction avoids non‑string values in the query.- Rendering
MainStatChartwith{data, period}is consistent with its interface.This looks good as‑is.
Also applies to: 44-47
apps/web/app/(app)/[emailAccountId]/stats/EmailAnalytics.tsx (1)
12-13: LGTM! Clean refactoring to BarListCard.The refactoring successfully consolidates the two separate BarList components into BarListCard components with tabs. The formatEmailItem helper cleanly adds Gmail search URLs to items, and the layout adjustment from 3 to 2 columns aligns with the new structure.
Also applies to: 41-47, 50-99
| const defaultFormatter = (value: any) => { | ||
| const date = new Date(value); | ||
|
|
||
| if (period === "year") { | ||
| return date.toLocaleDateString("en-US", { | ||
| year: "numeric", | ||
| }); | ||
| } | ||
|
|
||
| if (period === "month") { | ||
| return date.toLocaleDateString("en-US", { | ||
| month: "short", | ||
| year: "numeric", | ||
| }); | ||
| } | ||
|
|
||
| if (period === "week" || period === "day") { | ||
| return date.toLocaleDateString("en-US", { | ||
| month: "short", | ||
| day: "numeric", | ||
| }); | ||
| } | ||
|
|
||
| return date.toLocaleDateString("en-US", { | ||
| month: "short", | ||
| day: "numeric", | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Tooltip date formatting breaks for non‑date x‑axes
defaultFormatter and the tooltip both assume data[xAxisKey] is a date. For usages like RuleStatsChart (xAxisKey="group" with rule names), the tooltip header will render “Invalid Date”.
Consider guarding against non‑date values in the tooltip and falling back to the raw x value (or the same formatter used on the axis). For example:
- const data = payload[0];
- const date = new Date(data.payload[xAxisKey]);
-
- 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" };
- }
+ const dataPoint = payload[0];
+ const rawValue = dataPoint.payload[xAxisKey];
+ const date = new Date(rawValue);
+
+ let headerLabel: string;
+ if (!Number.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" };
+ }
+ headerLabel = date.toLocaleDateString("en-US", dateFormat);
+ } else {
+ headerLabel = String(rawValue ?? "");
+ }
@@
- <p className="mb-2 font-medium">
- {date.toLocaleDateString("en-US", dateFormat)}
- </p>
+ <p className="mb-2 font-medium">{headerLabel}</p>This keeps nice date formatting when appropriate while avoiding broken tooltips for categorical axes.
Also applies to: 56-57, 95-135, 137-148
🤖 Prompt for AI Agents
In apps/web/app/(app)/[emailAccountId]/stats/NewBarChart.tsx around lines 27-54
(and also apply same changes at 56-57, 95-135, 137-148), the current
defaultFormatter and tooltip assume data[xAxisKey] is always a date which yields
"Invalid Date" for categorical x values; update the formatter and any tooltip
header logic to first detect if the incoming value is a valid date (e.g.,
attempt new Date(value) and check !isNaN(date.getTime())) and only apply the
date formatting branches when valid, otherwise return the raw x value (or call
the axis label formatter for consistency); ensure the tooltip uses this guarded
formatter for its header so categorical axes show the original string instead of
"Invalid Date."
| SELECT | ||
| DATE_TRUNC(${Prisma.raw(`'${period}'`)}, date) AS "startOfPeriod", | ||
| COUNT(*) AS "totalCount", | ||
| SUM(CASE WHEN inbox = true THEN 1 ELSE 0 END) AS "inboxCount", | ||
| SUM(CASE WHEN inbox = false THEN 1 ELSE 0 END) AS "notInbox", | ||
| SUM(CASE WHEN read = true THEN 1 ELSE 0 END) AS "readCount", | ||
| SUM(CASE WHEN read = false THEN 1 ELSE 0 END) AS unread, | ||
| SUM(CASE WHEN sent = true THEN 1 ELSE 0 END) AS "sentCount" | ||
| FROM "EmailMessage" | ||
| ${whereClause}${dateClause} | ||
| GROUP BY "startOfPeriod" | ||
| ORDER BY "startOfPeriod" | ||
| `; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify SQL injection protection in zodPeriod validation.
Line 53 uses Prisma.raw() with string interpolation of the period parameter. Even though period is validated by zodPeriod, this pattern is risky and could lead to SQL injection if the validation doesn't strictly whitelist allowed values (e.g., 'day', 'week', 'month', 'year').
Run the following script to verify that zodPeriod provides strict validation:
Consider refactoring to use a safer approach with a whitelist mapping:
+ // Safe period mapping
+ const periodMap: Record<string, string> = {
+ day: 'day',
+ week: 'week',
+ month: 'month',
+ year: 'year'
+ };
+
+ const safePeriod = periodMap[period];
+ if (!safePeriod) {
+ throw new Error('Invalid period');
+ }
+
return prisma.$queryRaw<StatsResult[]>`
SELECT
- DATE_TRUNC(${Prisma.raw(`'${period}'`)}, date) AS "startOfPeriod",
+ DATE_TRUNC(${Prisma.raw(`'${safePeriod}'`)}, date) AS "startOfPeriod",🏁 Script executed:
#!/bin/bash
# Description: Verify zodPeriod validation only allows safe period values
# Find the zodPeriod definition
ast-grep --pattern 'export const zodPeriod = $$$'
# Also search for any z.enum or z.literal that might define allowed periods
rg -n -A5 'zodPeriod' --type=tsLength of output: 2783
Alias inconsistency on line 58: unquoted unread should be quoted.
Line 53's use of Prisma.raw() with the period parameter is safe because zodPeriod enforces a strict whitelist via z.enum(["day", "week", "month", "year"]). This Zod validation prevents SQL injection.
However, line 58 uses an unquoted alias AS unread while all other aliases are quoted (e.g., "startOfPeriod", "totalCount"). Ensure consistency by quoting this alias: AS "unread".
🤖 Prompt for AI Agents
In apps/web/app/api/user/stats/by-period/route.ts around lines 52 to 64, the SQL
SELECT uses an unquoted alias "unread" as AS unread while all other aliases are
quoted; update the query to use a quoted alias (AS "unread") to match the
existing alias style and maintain consistency across the SELECT clause.
| const hasSubdomain = domain.split(".").length > 2; | ||
| const apexDomain = hasSubdomain | ||
| ? domain.split(".").slice(1).join(".") | ||
| : domain; |
There was a problem hiding this comment.
Subdomain detection logic is oversimplified.
The current logic assumes any domain with more than 2 parts (split by ".") has a subdomain, but this doesn't account for:
- Country-code TLDs (e.g.,
example.co.ukwould be treated as having a subdomain) - Single-part domains or invalid formats
Consider using a more robust approach or add validation:
function getFavicon(domain: string) {
+ // Handle invalid or empty domains
+ if (!domain || !domain.includes('.')) {
+ return null;
+ }
+
const hasSubdomain = domain.split(".").length > 2;
const apexDomain = hasSubdomain
? domain.split(".").slice(1).join(".")
: domain;
return `https://t1.gstatic.com/faviconV2?client=SOCIAL&type=FAVICON&fallback_opts=TYPE,SIZE,URL&url=http://${apexDomain}&size=64`;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/components/charts/DomainIcon.tsx around lines 6–9, the current
subdomain detection (splitting on ".") misclassifies domains with multi-part
TLDs (e.g., example.co.uk) and doesn't validate input; replace the naive split
with a proper domain parse using a public suffix aware library (e.g., tldts or
psl): validate the domain string first (non-empty, contains a dot), use the
library to extract hostname, domain (apex) and subdomain, and then derive
hasSubdomain from the parsed subdomain being non-empty; add a fallback that
treats malformed or single-label hosts as no subdomain to avoid runtime errors.
| export interface HorizontalBarChartItem { | ||
| name: string; | ||
| value: number; | ||
| href?: string; | ||
| target?: string; | ||
| icon?: string; | ||
| } |
There was a problem hiding this comment.
Fix type mismatch for icon property.
The icon property is typed as string but is rendered as JSX content at line 48 (<span className="text-base">{item.icon}</span>). This works for emoji strings but the typing doesn't reflect that it's intended for text-based icons rather than React components.
Apply this diff to clarify the type:
export interface HorizontalBarChartItem {
name: string;
value: number;
href?: string;
target?: string;
- icon?: string;
+ icon?: React.ReactNode;
}Or, if you specifically want only emoji/text strings, add a comment to clarify the intent:
export interface HorizontalBarChartItem {
name: string;
value: number;
href?: string;
target?: string;
+ // Icon should be an emoji or text character, not a React component
icon?: string;
}📝 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.
| export interface HorizontalBarChartItem { | |
| name: string; | |
| value: number; | |
| href?: string; | |
| target?: string; | |
| icon?: string; | |
| } | |
| export interface HorizontalBarChartItem { | |
| name: string; | |
| value: number; | |
| href?: string; | |
| target?: string; | |
| icon?: React.ReactNode; | |
| } |
🤖 Prompt for AI Agents
In apps/web/components/charts/HorizontalBarChart.tsx around lines 6–12, the icon
field is typed as string but is rendered as JSX at line 48; change the type to
reflect that it can be rendered (e.g., icon?: React.ReactNode or icon?:
JSX.Element | string) and update imports if needed (import React or the
appropriate JSX types), or if you intend icon to be strictly text/emoji, keep
string but add a clarifying comment and ensure usage only passes strings; update
the interface accordingly and adjust any callers to match the new type.
| if (onItemClick) { | ||
| return ( | ||
| <button | ||
| key={item.name} | ||
| type="button" | ||
| className="w-full flex items-center justify-between gap-4 group" | ||
| onClick={() => onItemClick(item)} | ||
| > | ||
| {content} | ||
| </button> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Consider adding aria-label for better accessibility.
The interactive button lacks an explicit accessible label. While the button contains text content (item.name), adding an aria-label would make the button's purpose clearer to screen reader users.
Apply this diff to improve accessibility:
<button
key={item.name}
type="button"
className="w-full flex items-center justify-between gap-4 group"
onClick={() => onItemClick(item)}
+ aria-label={`View details for ${item.name}`}
>
{content}
</button>📝 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.
| if (onItemClick) { | |
| return ( | |
| <button | |
| key={item.name} | |
| type="button" | |
| className="w-full flex items-center justify-between gap-4 group" | |
| onClick={() => onItemClick(item)} | |
| > | |
| {content} | |
| </button> | |
| ); | |
| } | |
| if (onItemClick) { | |
| return ( | |
| <button | |
| key={item.name} | |
| type="button" | |
| className="w-full flex items-center justify-between gap-4 group" | |
| onClick={() => onItemClick(item)} | |
| aria-label={`View details for ${item.name}`} | |
| > | |
| {content} | |
| </button> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In apps/web/components/charts/HorizontalBarChart.tsx around lines 81 to 92, the
interactive button rendered for each item lacks an explicit accessible label;
add an aria-label attribute to the button (e.g., aria-label={`Select
${item.name}`} or a more descriptive localized string) so screen readers have a
clear, unique label for each button while keeping the existing onClick, key, and
visual content unchanged.
| <Button | ||
| variant={isSelected ? "ghost" : "ghost"} | ||
| size="sm" | ||
| className={cn( | ||
| "text-left justify-start", | ||
| isSelected ? "font-bold" : "font-normal", | ||
| )} | ||
| onClick={() => onSelect?.(item)} | ||
| key={item.value} |
There was a problem hiding this comment.
Remove redundant ternary operator.
Line 24 uses the same "ghost" variant for both selected and unselected states. Either remove the ternary or use different variants to distinguish selection state more clearly beyond just font weight.
Apply this diff to simplify:
<Button
- variant={isSelected ? "ghost" : "ghost"}
+ variant="ghost"
size="sm"
className={cn(
"text-left justify-start",
isSelected ? "font-bold" : "font-normal",
)}
- onClick={() => onSelect?.(item)}
+ onClick={() => onSelect(item)}
key={item.value}
>Note: Also removed optional chaining on onSelect since it's a required prop.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/components/List.tsx around lines 23 to 31, the Button's variant prop
uses a redundant ternary that always returns "ghost" and the onClick uses
optional chaining on onSelect even though onSelect is required; remove the
ternary and set variant="ghost" directly (or pick a different variant for
selected state if you want visual distinction), and remove the optional chaining
so onClick calls onSelect(item) directly.
| className={cn( | ||
| tabSelectButtonVariants({ variant }), | ||
| target === "_blank" && "group flex items-center gap-1.5", | ||
| isSelected ? "" : "text-gray-500 hover:text-gray-700", |
There was a problem hiding this comment.
Redundant color styling conflicts with variant system.
The conditional text color isSelected ? "" : "text-gray-500 hover:text-gray-700" duplicates and potentially conflicts with the variant's existing color handling on lines 22-24, which already manages selected/unselected states via data-[selected=true] and data-[selected=false] pseudo-classes.
Consider removing this line to rely on the variant system:
tabSelectButtonVariants({ variant }),
target === "_blank" && "group flex items-center gap-1.5",
- isSelected ? "" : "text-gray-500 hover:text-gray-700",
)}📝 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.
| isSelected ? "" : "text-gray-500 hover:text-gray-700", | |
| tabSelectButtonVariants({ variant }), | |
| target === "_blank" && "group flex items-center gap-1.5", | |
| )} |
🤖 Prompt for AI Agents
In apps/web/components/TabSelect.tsx around line 77, the inline conditional
color classes `isSelected ? "" : "text-gray-500 hover:text-gray-700"` duplicate
and conflict with the component's variant-driven color handling
(data-[selected=true]/[false] on lines ~22-24); remove that conditional from the
className expression so the variant pseudo-class rules control
selected/unselected colors, and adjust the className concatenation to avoid
leaving extra spaces or empty strings (ensure classNames/utils call still
receives only deliberate class tokens).
| border: "hsl(var(--sidebar-border))", | ||
| ring: "hsl(var(--sidebar-ring))", | ||
| }, | ||
| // TODO: rename |
There was a problem hiding this comment.
Address the TODO: Rename the "new" color group.
The TODO comment indicates this color group needs a proper name before this PR is merged. A generic "new" namespace will cause confusion and maintenance issues.
Please provide a descriptive name that reflects the purpose of this color palette (e.g., "brand", "analytics", "dashboard", etc.).
🤖 Prompt for AI Agents
In apps/web/tailwind.config.js around line 96 the color group currently named
"new" must be renamed to a descriptive name (e.g., "brand", "dashboard",
"analytics") to avoid ambiguity; pick the name that matches this palette's
purpose, replace the "new" key in the tailwind config with that chosen name, and
then update all code references (class names, theme lookups, tests, and docs) to
use the new key (perform a repo-wide search-and-replace for "new:" and "new-"
style usages and run the build/test to verify no breakages).
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
Issue on line in apps/web/components/new-landing/common/Anchor.tsx:15:
Opening links with target="_blank" without rel="noopener noreferrer" enables tabnabbing. Consider adding rel="noopener noreferrer" whenever opening a new tab.
| target={newTab ? "_blank" : undefined} | |
| rel={newTab ? "noopener noreferrer" : undefined} | |
| className={cn("underline", className)} |
🚀 Reply to ask Macroscope to explain or update this suggestion.
👍 Helpful? React to give us feedback.
| onSetDateDropdown, | ||
| }: DatePickerWithRangeProps) { | ||
| const now = useMemo(() => new Date(), []); | ||
| const days = |
There was a problem hiding this comment.
All shows when dateRange is undefined. Consider only computing relativeDateLabel when both from and to are set, so the button falls back to Pick a date.
- : 0;
- const relativeDateLabel = getRelativeDateLabel(days);
+ : undefined;
+ const relativeDateLabel = days != null ? getRelativeDateLabel(days) : undefined;🚀 Reply to ask Macroscope to explain or update this suggestion.
👍 Helpful? React to give us feedback.
| hideIcon={hideIcons} | ||
| /> | ||
| <div className="absolute w-full left-0 bottom-0 pb-6 z-30"> | ||
| {tabs.find((d) => d.id === selected)?.data.length > 0 && ( |
There was a problem hiding this comment.
tabs.find(... )?.data.length can crash when no tab matches because .length is read on undefined. Consider guarding the result, e.g., compare (tabs.find(... )?.data?.length ?? 0) > 0.
| {tabs.find((d) => d.id === selected)?.data.length > 0 && ( | |
| {(tabs.find((d) => d.id === selected)?.data?.length ?? 0) > 0 && ( |
🚀 Reply to ask Macroscope to explain or update this suggestion.
👍 Helpful? React to give us feedback.
| /> | ||
| </ActionBar> | ||
| <div className="grid gap-2 sm:gap-4 mt-2 sm:mt-4"> | ||
| <StatsSummary |
There was a problem hiding this comment.
Selecting All should clear the date filter. Passing a now‑to‑now dateRange constrains results; consider passing undefined when All is selected so queries fetch all time and the UI stays consistent.
🚀 Reply to ask Macroscope to explain or update this suggestion.
👍 Helpful? React to give us feedback.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
10 issues found across 56 files
Prompt for AI agents (all 10 issues)
Understand the root cause of the following 10 issues and fix them.
<file name="apps/web/app/(app)/[emailAccountId]/assistant/RulesSelect.tsx">
<violation number="1" location="apps/web/app/(app)/[emailAccountId]/assistant/RulesSelect.tsx:16">
Mark the component as a client component before using `useSearchParams`/`useRouter`; otherwise Next.js will crash because these hooks are client-only.</violation>
</file>
<file name="apps/web/components/DatePickerWithRange.tsx">
<violation number="1" location="apps/web/components/DatePickerWithRange.tsx:44">
Memoizing the current date causes every quick-select range to be anchored to the initial render time, so choosing a preset later yields an incorrect range.</violation>
</file>
<file name="apps/web/components/charts/HorizontalBarChart.tsx">
<violation number="1" location="apps/web/components/charts/HorizontalBarChart.tsx:52">
Avoid rendering an <a> when the row is wrapped in a <button>; the current structure nests interactive elements and causes double activation.</violation>
<violation number="2" location="apps/web/components/charts/HorizontalBarChart.tsx:55">
Links that can open in a new tab need `rel="noopener noreferrer"` to prevent reverse-tabnabbing when `target="_blank"`.</violation>
</file>
<file name="apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx">
<violation number="1" location="apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx:38">
Storing the x-axis date as a timezone-less `yyyy-MM-dd` string makes browsers parse it as UTC, so chart labels shift by a day for users outside UTC. Preserve the timestamp (e.g., keep the ISO string with timezone) before passing it to `NewBarChart`.</violation>
</file>
<file name="apps/web/app/(app)/[emailAccountId]/stats/Stats.tsx">
<violation number="1" location="apps/web/app/(app)/[emailAccountId]/stats/Stats.tsx:122">
EmailAnalytics is no longer gated by `isAccountOwner`, so non‑owners can now fetch and view another mailbox’s sender/recipient analytics. Wrap this component back in the ownership check to prevent exposing private stats or triggering unauthorized API calls.</violation>
</file>
<file name="apps/web/app/(app)/[emailAccountId]/bulk-unsubscribe/BulkUnsubscribeSection.tsx">
<violation number="1" location="apps/web/app/(app)/[emailAccountId]/bulk-unsubscribe/BulkUnsubscribeSection.tsx:250">
Wrapping all of the bulk-unsubscribe toolbar controls inside ActionBar removes the flex-wrap that previously let them stack, so on mobile/tablet widths the toggle, search bar, filter, date range picker, and Load Stats button overflow and become unusable.</violation>
</file>
<file name="apps/web/app/(app)/[emailAccountId]/bulk-unsubscribe/BulkUnsubscribeDesktop.tsx">
<violation number="1" location="apps/web/app/(app)/[emailAccountId]/bulk-unsubscribe/BulkUnsubscribeDesktop.tsx:128">
The new `<Progress>` bars never announce their percentage because the Progress component discards the `value` prop, so Radix treats them as indeterminate; forward the `value` (and max) to `ProgressPrimitive.Root` before using it here to avoid the accessibility regression.</violation>
</file>
<file name="apps/web/components/charts/DomainIcon.tsx">
<violation number="1" location="apps/web/components/charts/DomainIcon.tsx:57">
Reset the `fallbackEnabled` state when the `domain` prop changes so that a new favicon is attempted instead of permanently showing the fallback after the first error.</violation>
</file>
<file name="apps/web/app/(app)/[emailAccountId]/stats/NewBarChart.tsx">
<violation number="1" location="apps/web/app/(app)/[emailAccountId]/stats/NewBarChart.tsx:99">
Guard the tooltip header so it only applies date formatting when the x-axis value can be parsed as a valid date, otherwise fall back to the raw value to avoid showing “Invalid Date” for categorical charts.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
|
|
||
| export function RulesSelect() { | ||
| const { data, isLoading, error } = useRules(); | ||
| const searchParams = useSearchParams(); |
There was a problem hiding this comment.
Mark the component as a client component before using useSearchParams/useRouter; otherwise Next.js will crash because these hooks are client-only.
Prompt for AI agents
Address the following comment on apps/web/app/(app)/[emailAccountId]/assistant/RulesSelect.tsx at line 16:
<comment>Mark the component as a client component before using `useSearchParams`/`useRouter`; otherwise Next.js will crash because these hooks are client-only.</comment>
<file context>
@@ -1,28 +1,69 @@
export function RulesSelect() {
const { data, isLoading, error } = useRules();
+ const searchParams = useSearchParams();
+ const router = useRouter();
+ const currentValue = searchParams.get("ruleId") || "all";
</file context>
| dateDropdown, | ||
| onSetDateDropdown, | ||
| }: DatePickerWithRangeProps) { | ||
| const now = useMemo(() => new Date(), []); |
There was a problem hiding this comment.
Memoizing the current date causes every quick-select range to be anchored to the initial render time, so choosing a preset later yields an incorrect range.
Prompt for AI agents
Address the following comment on apps/web/components/DatePickerWithRange.tsx at line 44:
<comment>Memoizing the current date causes every quick-select range to be anchored to the initial render time, so choosing a preset later yields an incorrect range.</comment>
<file context>
@@ -13,48 +12,97 @@ import {
+ dateDropdown,
+ onSetDateDropdown,
+}: DatePickerWithRangeProps) {
+ const now = useMemo(() => new Date(), []);
+ const days =
+ dateRange?.from && dateRange?.to
</file context>
| {item.href ? ( | ||
| <a | ||
| href={item.href} | ||
| target={item.target} |
There was a problem hiding this comment.
Links that can open in a new tab need rel="noopener noreferrer" to prevent reverse-tabnabbing when target="_blank".
Prompt for AI agents
Address the following comment on apps/web/components/charts/HorizontalBarChart.tsx at line 55:
<comment>Links that can open in a new tab need `rel="noopener noreferrer"` to prevent reverse-tabnabbing when `target="_blank"`.</comment>
<file context>
@@ -0,0 +1,105 @@
+ {item.href ? (
+ <a
+ href={item.href}
+ target={item.target}
+ className="text-sm text-gray-900 truncate block z-10 relative hover:underline"
+ >
</file context>
| ) : ( | ||
| <DomainIcon domain={domain} /> | ||
| ))} | ||
| {item.href ? ( |
There was a problem hiding this comment.
Avoid rendering an when the row is wrapped in a ; the current structure nests interactive elements and causes double activation.
Prompt for AI agents
Address the following comment on apps/web/components/charts/HorizontalBarChart.tsx at line 52:
<comment>Avoid rendering an <a> when the row is wrapped in a <button>; the current structure nests interactive elements and causes double activation.</comment>
<file context>
@@ -0,0 +1,105 @@
+ ) : (
+ <DomainIcon domain={domain} />
+ ))}
+ {item.href ? (
+ <a
+ href={item.href}
</file context>
| const chartData = React.useMemo(() => { | ||
| return props.data.result.map((item) => { | ||
| const date = parse(item.startOfPeriod, "MMM dd, yyyy", new Date()); | ||
| const dateStr = format(date, "yyyy-MM-dd"); |
There was a problem hiding this comment.
Storing the x-axis date as a timezone-less yyyy-MM-dd string makes browsers parse it as UTC, so chart labels shift by a day for users outside UTC. Preserve the timestamp (e.g., keep the ISO string with timezone) before passing it to NewBarChart.
Prompt for AI agents
Address the following comment on apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx at line 38:
<comment>Storing the x-axis date as a timezone-less `yyyy-MM-dd` string makes browsers parse it as UTC, so chart labels shift by a day for users outside UTC. Preserve the timestamp (e.g., keep the ISO string with timezone) before passing it to `NewBarChart`.</comment>
<file context>
@@ -0,0 +1,104 @@
+ const chartData = React.useMemo(() => {
+ return props.data.result.map((item) => {
+ const date = parse(item.startOfPeriod, "MMM dd, yyyy", new Date());
+ const dateStr = format(date, "yyyy-MM-dd");
+
+ return {
</file context>
| const dateStr = format(date, "yyyy-MM-dd"); | |
| const dateStr = date.toISOString(); |
| refreshInterval={refreshInterval} | ||
| period={period} | ||
| /> | ||
| <EmailAnalytics |
There was a problem hiding this comment.
EmailAnalytics is no longer gated by isAccountOwner, so non‑owners can now fetch and view another mailbox’s sender/recipient analytics. Wrap this component back in the ownership check to prevent exposing private stats or triggering unauthorized API calls.
Prompt for AI agents
Address the following comment on apps/web/app/(app)/[emailAccountId]/stats/Stats.tsx at line 122:
<comment>EmailAnalytics is no longer gated by `isAccountOwner`, so non‑owners can now fetch and view another mailbox’s sender/recipient analytics. Wrap this component back in the ownership check to prevent exposing private stats or triggering unauthorized API calls.</comment>
<file context>
@@ -71,63 +70,68 @@ export function Stats() {
+ refreshInterval={refreshInterval}
period={period}
+ />
+ <EmailAnalytics
+ dateRange={dateRange}
refreshInterval={refreshInterval}
</file context>
| ) | ||
| } | ||
| /> | ||
| <ActionBar rightContent={<LoadStatsButton />}> |
There was a problem hiding this comment.
Wrapping all of the bulk-unsubscribe toolbar controls inside ActionBar removes the flex-wrap that previously let them stack, so on mobile/tablet widths the toggle, search bar, filter, date range picker, and Load Stats button overflow and become unusable.
Prompt for AI agents
Address the following comment on apps/web/app/(app)/[emailAccountId]/bulk-unsubscribe/BulkUnsubscribeSection.tsx at line 250:
<comment>Wrapping all of the bulk-unsubscribe toolbar controls inside ActionBar removes the flex-wrap that previously let them stack, so on mobile/tablet widths the toggle, search bar, filter, date range picker, and Load Stats button overflow and become unusable.</comment>
<file context>
@@ -246,34 +247,34 @@ export function BulkUnsubscribe() {
- )
- }
- />
+ <ActionBar rightContent={<LoadStatsButton />}>
+ <div className="flex items-center justify-end gap-1">
+ <div className="">
</file context>
| <div className="flex items-center gap-4"> | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <Progress value={readPercentage} className="h-2 w-[150px]" /> |
There was a problem hiding this comment.
The new <Progress> bars never announce their percentage because the Progress component discards the value prop, so Radix treats them as indeterminate; forward the value (and max) to ProgressPrimitive.Root before using it here to avoid the accessibility regression.
Prompt for AI agents
Address the following comment on apps/web/app/(app)/[emailAccountId]/bulk-unsubscribe/BulkUnsubscribeDesktop.tsx at line 128:
<comment>The new `<Progress>` bars never announce their percentage because the Progress component discards the `value` prop, so Radix treats them as indeterminate; forward the `value` (and max) to `ProgressPrimitive.Root` before using it here to avoid the accessibility regression.</comment>
<file context>
@@ -117,27 +122,36 @@ export function BulkUnsubscribeRowDesktop({
+ <div className="flex items-center gap-4">
+ <Tooltip>
+ <TooltipTrigger asChild>
+ <Progress value={readPercentage} className="h-2 w-[150px]" />
+ </TooltipTrigger>
+ <TooltipContent>
</file context>
|
|
||
| export function DomainIcon({ domain }: DomainIconProps) { | ||
| const domainFavicon = getFavicon(domain); | ||
| const [fallbackEnabled, setFallbackEnabled] = useState(false); |
There was a problem hiding this comment.
Reset the fallbackEnabled state when the domain prop changes so that a new favicon is attempted instead of permanently showing the fallback after the first error.
Prompt for AI agents
Address the following comment on apps/web/components/charts/DomainIcon.tsx at line 57:
<comment>Reset the `fallbackEnabled` state when the `domain` prop changes so that a new favicon is attempted instead of permanently showing the fallback after the first error.</comment>
<file context>
@@ -0,0 +1,75 @@
+
+export function DomainIcon({ domain }: DomainIconProps) {
+ const domainFavicon = getFavicon(domain);
+ const [fallbackEnabled, setFallbackEnabled] = useState(false);
+
+ return (
</file context>
| content={({ active, payload }) => { | ||
| if (!active || !payload?.length) return null; | ||
| const data = payload[0]; | ||
| const date = new Date(data.payload[xAxisKey]); |
There was a problem hiding this comment.
Guard the tooltip header so it only applies date formatting when the x-axis value can be parsed as a valid date, otherwise fall back to the raw value to avoid showing “Invalid Date” for categorical charts.
Prompt for AI agents
Address the following comment on apps/web/app/(app)/[emailAccountId]/stats/NewBarChart.tsx at line 99:
<comment>Guard the tooltip header so it only applies date formatting when the x-axis value can be parsed as a valid date, otherwise fall back to the raw value to avoid showing “Invalid Date” for categorical charts.</comment>
<file context>
@@ -0,0 +1,152 @@
+ content={({ active, payload }) => {
+ if (!active || !payload?.length) return null;
+ const data = payload[0];
+ const date = new Date(data.payload[xAxisKey]);
+
+ let dateFormat: Intl.DateTimeFormatOptions;
</file context>
3 participants
Replace Tremor charts with
NewBarChart, restructure stats pages withMainStatChart,BarListCard, and presetDatePickerWithRange, and update Tailwind to remove Tremor tokens to support Common Topics workSwitch charting to
NewBarChartacross analytics, introduceMainStatChartandBarListCard, refactorActionBarto layout-only, add preset-drivenDatePickerWithRange, remove Tremor dependencies and tokens, and add favicon support viat1.gstatic.com. Key entry points: [file:apps/web/app/(app)/[emailAccountId]/stats/Stats.tsx], [file:apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx], [file:apps/web/app/(app)/[emailAccountId]/stats/NewBarChart.tsx], and [file:apps/web/tailwind.config.js].📍Where to Start
Start with
Statspage composition in Stats.tsx, then review chart foundations inNewBarChart([file:apps/web/app/(app)/[emailAccountId]/stats/NewBarChart.tsx]) and summary logic inMainStatChart([file:apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx]); validate theme changes in [file:apps/web/tailwind.config.js].📊 Macroscope summarized 480449f. 46 files reviewed, 41 issues evaluated, 30 issues filtered, 4 comments posted
🗂️ Filtered Issues
apps/web/app/(app)/[emailAccountId]/bulk-unsubscribe/BulkUnsubscribeSection.tsx — 0 comments posted, 3 evaluated, 2 filtered
readPercentageandarchivedPercentageare computed by dividing byitem.valuewithout guarding against0. Ifitem.valueis0, both(item.readEmails / item.value) * 100and((item.value - item.inboxEmails) / item.value) * 100will yieldInfinityorNaN, which then propagates to UI components (e.g., progress bars and percentage text), causing broken rendering and potentially invalidvalueprops. Add a zero-check and fall back to0or skip rendering whenitem.value === 0. [ Out of scope ]sortColumn === "emails", the sorting iteratee returnsundefined, sosortBydoesn’t sort by the intended emails count. This yields an unsorted or implementation-defined order rather than sorting byitem.value. Useitem.value(the emails count) as the iteratee for the"emails"case. [ Out of scope ]apps/web/app/(app)/[emailAccountId]/stats/DetailedStatsFilter.tsx — 0 comments posted, 1 evaluated, 1 filtered
DetailedStatsFiltersetsDropdownMenuto a controlled open state whenkeepOpenOnSelectis true, but itsonOpenChangehandler ignores theopenargument and only ever setsisOpentotrueif it was previouslyfalse. This prevents normal close actions (e.g., pressing Escape or clicking the trigger again) from closing the menu; the menu can remain stuck open unlessonInteractOutsidefires. Use theopenparameter to set state (setIsOpen(open)) or otherwise allow close events when appropriate. [ Low confidence ]apps/web/app/(app)/[emailAccountId]/stats/MainStatChart.tsx — 0 comments posted, 1 evaluated, 1 filtered
format(date, "yyyy-MM-dd")is called ondateparsed fromitem.startOfPeriodwithout validating the parse result. Ifitem.startOfPerioddoes not match"MMM dd, yyyy",parse(...)yields an Invalid Date anddate-fnsformat(...)will throw aRangeError: Invalid time value. Add validation (e.g.,isValid(date)) or a fallback before formatting. [ Low confidence ]apps/web/app/(app)/[emailAccountId]/stats/NewBarChart.tsx — 0 comments posted, 2 evaluated, 2 filtered
NewBarChartassumesxAxisKeyvalues are valid dates.new Date(value)followed bytoLocaleDateString(...)will throwRangeError: Invalid time valueifvalueis missing or unparsable. This occurs in the default tick formatter and in the tooltip (new Date(data.payload[xAxisKey])). Add validation and fallback formatting, or guard against missing/invalidxAxisKeyvalues. [ Low confidence ]config[key].coloris accessed without existence checks while iteratingkeysderived fromdataKeys || Object.keys(config). IfdataKeysincludes a key not present inconfig, accessingconfig[key].colorwill throw at runtime. Guard withconfig[key] ? config[key].color : fallback, validatedataKeysagainstconfig, or derivekeyssolely fromconfig. Occurs in gradientstopColorand in<Bar color={...}>. [ Low confidence ]apps/web/app/(app)/[emailAccountId]/stats/NewsletterModal.tsx — 0 comments posted, 3 evaluated, 2 filtered
Button(renders as a native<button>whenasChildis false) contains an<a>anchor inside it (lines59-67). Nesting interactive controls is invalid HTML and can cause broken keyboard/focus behavior and inconsistent click handling. UseButtonwithasChildand render the anchor as the root (e.g.,<Button asChild><Link ... /></Button>) or make the anchor the primary element without wrapping it in a button. [ Out of scope ]newsletter.autoArchivedis truthy, the UI always shows aView Skip Inbox Filterlink built withgetGmailFilterSettingsUrl(userEmail)(lines82-91). For non-Gmail providers (e.g., Outlook), this link is incorrect and misleading. Gate this by provider (show Gmail link only for Google accounts) or render the appropriate settings destination per provider. [ Out of scope ]apps/web/app/api/user/stats/by-period/route.ts — 0 comments posted, 1 evaluated, 1 filtered
getEmailStatsByPerioduses truthy checks forfromDateandtoDate(if (fromDate),if (toDate)), which will skip valid timestamp values of0(Unix epoch). SincestatsByWeekParamsallows any number viaz.coerce.number().nullish(),0is an explicitly valid input and should be included. Use explicit null/undefined checks (e.g.,if (fromDate != null)) to avoid dropping epoch filters. [ Out of scope ]apps/web/components/DatePickerWithRange.tsx — 1 comment posted, 3 evaluated, 2 filtered
valueof"0") now setsdateRangeto{ from: subDays(now, 0), to: now }(i.e., a single day) instead of clearing the range. Previously, "All" cleared the date range (undefined). This breaks contract parity and produces inconsistent semantics: the label shows"All"while the selection is actually a one-day range. [ Low confidence ]valueinselectOptionswill passNaNtoNumber.parseInt(value), resulting insubDays(now, NaN)which yields an invalidDate. That invalidDateis set intodateRange({ from: Invalid Date, to: now }), and then passed to<Calendar selected={dateRange}>, risking runtime errors or inconsistent rendering in the date picker. [ Low confidence ]apps/web/components/PageHeader.tsx — 0 comments posted, 1 evaluated, 1 filtered
video && (video.youtubeVideoId || video.muxPlaybackId)relies on JavaScript truthiness to decide whether to render<WatchVideo />. This will render the button when eitheryoutubeVideoIdormuxPlaybackIdis a non-empty string, including whitespace-only values (e.g.," "), which are truthy but invalid IDs. That can cause downstream runtime errors or a broken player when<OnboardingDialogContent>receives an invalid ID. Use explicit validation (e.g., trim and non-empty checks) before rendering:const hasValidId = !!(video.youtubeVideoId?.trim() || video.muxPlaybackId?.trim());and guard onhasValidId. [ Low confidence ]apps/web/components/ProgressPanel.tsx — 0 comments posted, 2 evaluated, 2 filtered
progressis not clamped to the [0, 100] range. IfremainingItemsis greater thantotalItems,totalProcessedbecomes negative andprogressbecomes negative; ifremainingItemsis negative ortotalItemsis smaller thantotalProcessed,progresscan exceed 100. This yields incorrect UI and out-of-bounds indicator motion when passed to<Progress value={progress} />. Clampprogressto[0, 100]before rendering. [ Low confidence ]<div>is rendered inside an inline<span>(<span> ... <div className="flex items-center gap-1"> ... </div> ... </span>). This produces malformed markup and may lead to inconsistent layout/semantics across browsers and screen readers. Replace the outer<span>with a<div>or change the inner container to a<span>. [ Out of scope ]apps/web/components/TabSelect.tsx — 0 comments posted, 4 evaluated, 4 filtered
hrefis provided,AsisLinkwhich renders an<a>element, and a<button>is rendered inside it. Nesting interactive elements (<button>inside<a>) violates accessibility and can cause unexpected click/keyboard behavior. Consider making the clickable element a single<a>styled as a button, or handling selection/navigation on one element only. [ Out of scope ]hrefis falsy,Asbecomes adiv, buthrefandtargetprops are still applied (href={href ?? "#"}andtarget={target ?? undefined}), resulting in invalid DOM attributes on a<div>. This can produce console warnings and malformed semantics (a non-interactive element with link attributes). Guard these props so they are only passed whenAsisLink, or conditionally render attributes based onhrefpresence. [ Out of scope ]target="_blank") withoutrel="noopener noreferrer"is a security/performance issue (tabnabbing). Addrel="noopener noreferrer"whentarget="_blank". [ Out of scope ]ArrowUpRight) renders whentarget === "_blank"even ifhrefis absent (soAsis adiv). This suggests an external link but results in a non-link element withtargetattr incorrectly applied, confusing users and a11y. Gate the icon andtargetby the presence ofhref. [ Out of scope ]apps/web/components/charts/DomainIcon.tsx — 0 comments posted, 1 evaluated, 1 filtered
getFaviconattempts to derive the apex domain by removing only the first label when it detects a subdomain (lines6-10in code object0). For multi-level subdomains (e.g.,a.b.example.com), it returnsb.example.comrather thanexample.com. For special public suffixes (e.g.,example.co.ukwithwww), this heuristic can produce incorrect hosts. This yields incorrect favicon URLs and avoidableonErrorfallbacks. Consider using a proper public suffix list or leave the domain unchanged. [ Low confidence ]apps/web/components/charts/HorizontalBarChart.tsx — 0 comments posted, 4 evaluated, 4 filtered
item.nameis an email missing the domain part (e.g.,"user@") or contains multiple@,domaincomputed inHorizontalBarChart(lines33-35in code object8) can be empty or invalid, leadingDomainIconto build a favicon URL withurl=http://(no host). This reliably triggers an error andonErrorfallback on each render. Add a guard to skip favicon fetch when the extracted domain is empty or invalid. [ Low confidence ]target="_blank") are rendered withoutrel="noopener noreferrer"(lines53-59in code object8). This exposes users to reverse tabnabbing and leaves the opener reference accessible to the new page. Addrel="noopener noreferrer"whentarget==="_blank". [ Low confidence ]HorizontalBarChartrenders a<button>container whenonItemClickis provided (lines83-91in code object8), butcontentmay contain an interactive<a>element whenitem.hrefis set (lines53-59in code object8). This nests an interactive control inside another interactive control, which is invalid HTML, can cause conflicting click/keyboard behaviors, and may lead to double-handling (both navigation andonItemClickfiring). Use either a single interactive element per item or conditionally avoid wrapping link content inside a button. [ Low confidence ]item.name(lines84and96in code object8). Ifdatacontains duplicate names, keys collide, causing incorrect reconciliation, stale UI state, or event handler mix-ups. Use a stable unique key (e.g., an ID or index combined with name) or validate uniqueness. [ Low confidence ]apps/web/components/new-landing/BrandScroller.tsx — 0 comments posted, 2 evaluated, 2 filtered
classNameon line 23 contains an invalid Tailwind arbitrary style token:[gap:var(--gap))]has an extra)which prevents Tailwind from generating the rule. As a result, the intendedgap: var(--gap)will not apply, breaking spacing between items in the scroller. It should be[gap:var(--gap)]. [ Out of scope ]altas the Reactkeyfor brand items (line 33) is unsafe becausealtvalues are not guaranteed unique across a customizablebrandList. Duplicate keys cause React reconciliation issues (items may be merged or updated incorrectly), leading to visual glitches during animation. Use a guaranteed-unique identifier (e.g., an index scoped to the list or a stable uniqueid) or enforce uniqueness ofaltupstream. [ Low confidence ]apps/web/components/new-landing/UnicornScene.tsx — 0 comments posted, 3 evaluated, 1 filtered
UnicornStudio.init()if the CDN script respects an existing global and does not overwritewindow.UnicornStudiobecause it was set to the flag object earlier. In that case,UnicornStudiowould still refer to the flag object without aninitmethod, andscript.onloadwill attemptUnicornStudio.init()leading to a runtime error. [ Low confidence ]apps/web/components/new-landing/common/Badge.tsx — 0 comments posted, 1 evaluated, 1 filtered
icon || nullcan silently drop validReact.ReactNodevalues like the number0. React supports rendering numbers, but because0is falsy,icon || nullwill rendernullinstead of0. This causes unexpected loss of output when a numeric icon or count of0is passed. Prefer{icon}(React ignoresundefined/nullautomatically) or an explicit check foricon !== undefined && icon !== nullto avoid dropping0. [ Out of scope ]apps/web/components/new-landing/sections/Pricing.tsx — 0 comments posted, 1 evaluated, 1 filtered
Pricing, theRadioGroup’sclassNamecontains an invalid RGBA alpha valuergba(0,0,0,0.0.07)within the arbitrary shadow class:shadow-[0_0_7px_0_rgba(0,0,0,0.0.07)]. CSS numbers cannot have multiple decimal points, so this likely results in the entirebox-shadowbeing ignored by the browser. Use a valid value likergba(0,0,0,0.07). [ Out of scope ]apps/web/components/ui/progress.tsx — 0 comments posted, 1 evaluated, 1 filtered
Progressdoes not defensively clampvalueto[0, 100]. When given out-of-range values (e.g., negative or >100), the computed transformtranslateX(-${100 - (value || 0)}%)can produce unexpected motion beyond the track. Clampvalueto[0, 100]before computing the transform to ensure consistent visuals. [ Low confidence ]apps/web/next.config.ts — 0 comments posted, 1 evaluated, 1 filtered
Access-Control-Allow-Originheader globally to a fixed valueenv.NEXT_PUBLIC_BASE_URLon all routes (source: '/(.*)') can cause CORS failures for requests originating from any other valid deployment domain (e.g., preview/staging URLs). When the browser origin doesn’t exactly matchenv.NEXT_PUBLIC_BASE_URL, cross-origin requests to Next.js API routes or assets will be blocked by CORS, breaking functionality in those environments. Consider reflecting the request’sOriginwhen appropriate or scoping CORS headers only to API endpoints that require them. [ Low confidence ]Summary by CodeRabbit
Release Notes
New Features
Improvements
Removals
✏️ Tip: You can customize this high-level summary in your review settings.