fix: Add unsubscribe functionality to NewsletterModal
#210
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,50 +1,64 @@ | ||||||
| import { MoreDropdown } from "@/app/(app)/bulk-unsubscribe/common"; | ||||||
| import { useUnsubscribe } from "@/app/(app)/bulk-unsubscribe/hooks"; | ||||||
| import { Row } from "@/app/(app)/bulk-unsubscribe/types"; | ||||||
| import { getDateRangeParams } from "@/app/(app)/stats/params"; | ||||||
| import type { ThreadsResponse } from "@/app/api/google/threads/controller"; | ||||||
| import type { | ||||||
| SenderEmailsQuery, | ||||||
| SenderEmailsResponse, | ||||||
| } from "@/app/api/user/stats/sender-emails/route"; | ||||||
| import { AlertBasic } from "@/components/Alert"; | ||||||
| import { LoadingContent } from "@/components/LoadingContent"; | ||||||
| import { usePremium } from "@/components/PremiumAlert"; | ||||||
| import { Tooltip } from "@/components/Tooltip"; | ||||||
| import { SectionHeader } from "@/components/Typography"; | ||||||
| import { EmailList } from "@/components/email-list/EmailList"; | ||||||
| import { Button } from "@/components/ui/button"; | ||||||
| import { | ||||||
| Dialog, | ||||||
| DialogContent, | ||||||
| DialogHeader, | ||||||
| DialogTitle, | ||||||
| } from "@/components/ui/dialog"; | ||||||
| import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs"; | ||||||
| import { useLabels } from "@/hooks/useLabels"; | ||||||
| import { onAutoArchive } from "@/utils/actions/client"; | ||||||
| import { getGmailFilterSettingsUrl } from "@/utils/url"; | ||||||
| import type { ZodPeriod } from "@inboxzero/tinybird"; | ||||||
| import { BarChart } from "@tremor/react"; | ||||||
| import { ExternalLinkIcon } from "lucide-react"; | ||||||
| import { useSession } from "next-auth/react"; | ||||||
| import Link from "next/link"; | ||||||
| import { usePostHog } from "posthog-js/react"; | ||||||
| import type { DateRange } from "react-day-picker"; | ||||||
| import useSWR from "swr"; | ||||||
|
|
||||||
| export function NewsletterModal(props: { | ||||||
| newsletter: Pick<Row, "name" | "lastUnsubscribeLink" | "autoArchived">; | ||||||
| onClose: (isOpen: boolean) => void; | ||||||
| refreshInterval?: number; | ||||||
| mutate?: () => Promise<any>; | ||||||
|
There was a problem hiding this comment. Avoid Using The use of - mutate?: () => Promise<any>;
+ mutate?: () => Promise<void>;Committable suggestion
Suggested change
ToolsBiome
There was a problem hiding this comment. I agree bot. I tried my best to make the suggestion of The issue here is that the The only fixes I could think of was to:
I think this will need refactoring in the future regardless. Would be happy to have a look at this later. There was a problem hiding this comment. @aryanprince, thanks for sharing your insights and potential solutions. It seems like a refactoring effort could indeed be beneficial to address the scope of the
Comment on lines
+37
to
+40
There was a problem hiding this comment. Approve newsletter prop change, but consider specifying mutate type The change to make the The The mutate?: () => Promise<unknown>;This maintains type safety while allowing flexibility in the return type. ToolsBiome
|
||||||
| }) { | ||||||
| const { newsletter, refreshInterval, onClose } = props; | ||||||
| const mutate = props.mutate || (() => Promise.resolve()); // Set a default value for mutate if it's not provided | ||||||
|
|
||||||
| const session = useSession(); | ||||||
| const email = session.data?.user.email; | ||||||
|
|
||||||
| const { userLabels } = useLabels(); | ||||||
|
|
||||||
| const { hasUnsubscribeAccess, mutate: refetchPremium } = usePremium(); | ||||||
|
|
||||||
| const posthog = usePostHog(); | ||||||
|
|
||||||
| const { unsubscribeLoading, onUnsubscribe } = useUnsubscribe({ | ||||||
| item: newsletter, | ||||||
| hasUnsubscribeAccess, | ||||||
| mutate, | ||||||
| posthog, | ||||||
| refetchPremium, | ||||||
| }); | ||||||
|
|
||||||
| return ( | ||||||
| <Dialog open={!!newsletter} onOpenChange={onClose}> | ||||||
| <DialogContent className="max-h-screen overflow-x-scroll overflow-y-scroll lg:min-w-[880px] xl:min-w-[1280px]"> | ||||||
|
|
@@ -55,13 +69,21 @@ export function NewsletterModal(props: { | |||||
| </DialogHeader> | ||||||
|
|
||||||
| <div className="flex space-x-2"> | ||||||
| <Button size="sm" variant="outline" disabled={unsubscribeLoading}> | ||||||
| <a | ||||||
| href={newsletter.lastUnsubscribeLink || undefined} | ||||||
| target="_blank" | ||||||
| rel="noreferrer" | ||||||
| onClick={onUnsubscribe} | ||||||
| > | ||||||
| {unsubscribeLoading ? ( | ||||||
| <div className="flex cursor-not-allowed items-center opacity-50"> | ||||||
| <Button loading={true} /> | ||||||
| <span>Unsubscribing...</span> | ||||||
| </div> | ||||||
| ) : ( | ||||||
| <span>Unsubscribe</span> | ||||||
| )} | ||||||
| </a> | ||||||
| </Button> | ||||||
| <Tooltip content="Auto archive emails using Gmail filters"> | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you need to add
{openedNewsletter && (?I think it stops the animation when it's done like that. Or was this added to fix a bug?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reason
This is cus the
<NewsletterModal />component now requires (non-optional) theopenedNewsletterprop, which was previously optional.The reason for this change is that the component now uses the existing
useUnsubscribeButtonhook, which does need a requiredopenedNewsletterprop.Initially, I tried keeping openedNewsletter as optional (to have less diffs like the one you brought up here), but that would have required introducing a second hook for unsubscribing, which would complicate the logic unnecessarily.
Minor Benefit
Additionally, making
openedNewslettera required prop also ensures that the<NewsletterModal />is only rendered when all necessary data is present, making it more robust and predictable in terms of when it renders throughout the app.Quick Demo
Also, below GIF shows that the animations are still intact. No issues with that according to my testing.