-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Mobile partner messaging fixes #3459
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
e45417d
3d8932f
d79b994
5cd2580
99161e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -16,18 +16,28 @@ import { PartnerInfoGroup } from "@/ui/partners/partner-info-group"; | |||||||||||||||||
| import { PartnerInfoSection } from "@/ui/partners/partner-info-section"; | ||||||||||||||||||
| import { PartnerInfoStats } from "@/ui/partners/partner-info-stats"; | ||||||||||||||||||
| import { X } from "@/ui/shared/icons"; | ||||||||||||||||||
| import { Button } from "@dub/ui"; | ||||||||||||||||||
| import { Button, useMediaQuery } from "@dub/ui"; | ||||||||||||||||||
| import { ChevronLeft } from "@dub/ui/icons"; | ||||||||||||||||||
| import { OG_AVATAR_URL, cn } from "@dub/utils"; | ||||||||||||||||||
| import { useAction } from "next-safe-action/hooks"; | ||||||||||||||||||
| import Link from "next/link"; | ||||||||||||||||||
| import { redirect, useParams } from "next/navigation"; | ||||||||||||||||||
| import { useState } from "react"; | ||||||||||||||||||
| import { | ||||||||||||||||||
| usePathname, | ||||||||||||||||||
| redirect, | ||||||||||||||||||
| useParams, | ||||||||||||||||||
| useRouter, | ||||||||||||||||||
| useSearchParams, | ||||||||||||||||||
| } from "next/navigation"; | ||||||||||||||||||
| import { useEffect, useState } from "react"; | ||||||||||||||||||
| import { toast } from "sonner"; | ||||||||||||||||||
| import { v4 as uuid } from "uuid"; | ||||||||||||||||||
|
|
||||||||||||||||||
| export function ProgramMessagesPartnerPageClient() { | ||||||||||||||||||
| const { id: workspaceId, slug: workspaceSlug } = useWorkspace(); | ||||||||||||||||||
| const router = useRouter(); | ||||||||||||||||||
| const pathname = usePathname(); | ||||||||||||||||||
| const searchParams = useSearchParams(); | ||||||||||||||||||
| const { width } = useMediaQuery(); | ||||||||||||||||||
|
|
||||||||||||||||||
| const { partnerId } = useParams() as { partnerId: string }; | ||||||||||||||||||
| const { user } = useUser(); | ||||||||||||||||||
|
|
@@ -71,17 +81,40 @@ export function ProgramMessagesPartnerPageClient() { | |||||||||||||||||
| }, | ||||||||||||||||||
| }); | ||||||||||||||||||
|
|
||||||||||||||||||
| const partner = partnerMessages?.[0]?.partner; | ||||||||||||||||||
| const messages = partnerMessages?.[0]?.messages; | ||||||||||||||||||
| const { setCurrentPanel, targetThreadId } = useMessagesContext(); | ||||||||||||||||||
| const isThreadTransitioning = | ||||||||||||||||||
| targetThreadId !== null && targetThreadId !== partnerId; | ||||||||||||||||||
|
|
||||||||||||||||||
| const activeThread = | ||||||||||||||||||
| !isThreadTransitioning && partnerMessages?.[0]?.partner.id === partnerId | ||||||||||||||||||
| ? partnerMessages[0] | ||||||||||||||||||
| : undefined; | ||||||||||||||||||
| const partner = activeThread?.partner; | ||||||||||||||||||
| const messages = activeThread?.messages; | ||||||||||||||||||
|
|
||||||||||||||||||
| const { executeAsync: sendMessage } = useAction(messagePartnerAction, { | ||||||||||||||||||
| onError({ error }) { | ||||||||||||||||||
| toast.error(parseActionError(error, "Failed to send message")); | ||||||||||||||||||
| }, | ||||||||||||||||||
| }); | ||||||||||||||||||
|
|
||||||||||||||||||
| const { setCurrentPanel } = useMessagesContext(); | ||||||||||||||||||
| const [isRightPanelOpen, setIsRightPanelOpen] = useState(true); | ||||||||||||||||||
| const [isRightPanelOpen, setIsRightPanelOpen] = useState(false); | ||||||||||||||||||
| const shouldAutoFocusComposer = searchParams.get("new") === "1"; | ||||||||||||||||||
|
|
||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||
| if (typeof width !== "number") return; | ||||||||||||||||||
| setIsRightPanelOpen(width >= 960); | ||||||||||||||||||
| }, [partnerId, width]); | ||||||||||||||||||
|
Comment on lines
+104
to
+107
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right panel effect re-fires on every If a user manually closes the right panel on a wide screen (≥960px), any subsequent resize event (even a 1px change) will reopen it because the effect unconditionally sets Consider tracking whether the user has manually toggled the panel and skipping the auto-set in that case, or only running this logic on ♻️ Possible approach: only auto-set on partnerId change useEffect(() => {
if (typeof width !== "number") return;
setIsRightPanelOpen(width >= 960);
- }, [partnerId, width]);
+ }, [partnerId]); // eslint-disable-line react-hooks/exhaustive-depsThis would set the initial panel state based on current viewport when switching partners, without overriding manual toggles during resizes. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
|
|
||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||
| if (!shouldAutoFocusComposer) return; | ||||||||||||||||||
|
|
||||||||||||||||||
| const nextSearchParams = new URLSearchParams(searchParams.toString()); | ||||||||||||||||||
| nextSearchParams.delete("new"); | ||||||||||||||||||
| const nextSearch = nextSearchParams.toString(); | ||||||||||||||||||
|
|
||||||||||||||||||
| router.replace(nextSearch ? `${pathname}?${nextSearch}` : pathname); | ||||||||||||||||||
| }, [pathname, router, searchParams, shouldAutoFocusComposer]); | ||||||||||||||||||
|
|
||||||||||||||||||
| if (errorMessages) redirect(`/${workspaceSlug}/program/messages`); | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -144,6 +177,7 @@ export function ProgramMessagesPartnerPageClient() { | |||||||||||||||||
| currentUserId={user?.id || ""} | ||||||||||||||||||
| program={program} | ||||||||||||||||||
| partner={partner} | ||||||||||||||||||
| autoFocusComposer={shouldAutoFocusComposer} | ||||||||||||||||||
| onSendMessage={async (message) => { | ||||||||||||||||||
| const createdAt = new Date(); | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,7 @@ import useProgram from "@/lib/swr/use-program"; | |
| import { useProgramReferralsCount } from "@/lib/swr/use-program-referrals-count"; | ||
| import useWorkspace from "@/lib/swr/use-workspace"; | ||
| import useWorkspaces from "@/lib/swr/use-workspaces"; | ||
| import { useRouterStuff } from "@dub/ui"; | ||
| import { useMediaQuery, useRouterStuff } from "@dub/ui"; | ||
| import { | ||
| Bell, | ||
| Brush, | ||
|
|
@@ -498,6 +498,7 @@ export function AppSidebarNav({ | |
| }) { | ||
| const { slug: paramsSlug } = useParams() as { slug?: string }; | ||
| const pathname = usePathname(); | ||
| const { width, isMobile } = useMediaQuery(); | ||
| const { getQueryString } = useRouterStuff(); | ||
| const { data: session, status } = useSession(); | ||
| const { plan, defaultProgramId } = useWorkspace(); | ||
|
|
@@ -547,18 +548,25 @@ export function AppSidebarNav({ | |
| session?.user?.["defaultWorkspace"]; | ||
|
|
||
| const currentArea = useMemo(() => { | ||
| const focusedProgramPath = | ||
| pathname.includes("/program/campaigns/") || | ||
| pathname.includes("/program/messages/") || | ||
| pathname.endsWith("/program/payouts/success"); | ||
| const isMobileDrawerViewport = | ||
| typeof width === "number" ? width < 768 : isMobile; | ||
|
Comment on lines
+555
to
+556
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # Find the useMediaQuery hook definition
rg -n "useMediaQuery" --type=ts --type=tsx -A 5 -B 2 | head -100Repository: dubinc/dub Length of output: 81 🏁 Script executed: # Examine the specific file and context around lines 555-556
cat -n apps/web/ui/layout/sidebar/app-sidebar-nav.tsx | sed -n '545,570p'Repository: dubinc/dub Length of output: 1049 🏁 Script executed: # Look for the currentArea variable and its dependencies
rg -n "currentArea" apps/web/ui/layout/sidebar/app-sidebar-nav.tsx -B 2 -A 2Repository: dubinc/dub Length of output: 1471 🏁 Script executed: # Find useMediaQuery hook definition and where width/isMobile come from
rg -n "const.*useMediaQuery\|function useMediaQuery\|export.*useMediaQuery" --type=ts -A 10 | head -50Repository: dubinc/dub Length of output: 36 🏁 Script executed: # Search in the same file to see where width and isMobile are declared
cat -n apps/web/ui/layout/sidebar/app-sidebar-nav.tsx | head -100Repository: dubinc/dub Length of output: 3737 🏁 Script executed: # Look for imports and variable declarations in the file
cat -n apps/web/ui/layout/sidebar/app-sidebar-nav.tsx | grep -n "import\|width\|isMobile" | head -30Repository: dubinc/dub Length of output: 1929 🏁 Script executed: # Find the useMediaQuery hook implementation in `@dub/ui`
find . -type f -name "*.ts" -o -name "*.tsx" | xargs grep -l "export.*useMediaQuery\|function useMediaQuery" | head -10Repository: dubinc/dub Length of output: 96 🏁 Script executed: # Search more broadly for useMediaQuery definition
rg -n "useMediaQuery" --type=ts -A 20 | grep -A 20 "function\|export\|useState\|initialValue" | head -80Repository: dubinc/dub Length of output: 4959 🏁 Script executed: # Examine the useMediaQuery hook implementation
cat -n ./packages/ui/src/hooks/use-media-query.tsRepository: dubinc/dub Length of output: 1738 Potential one-frame flash on mobile for focused program paths. The Consider initializing the guard optimistically (e.g., treat an unresolved 🤖 Prompt for AI Agents |
||
|
|
||
| return pathname.startsWith("/account/settings") | ||
| ? "userSettings" | ||
| : pathname.startsWith(`/${slug}/settings`) | ||
| ? "workspaceSettings" | ||
| : pathname.includes("/program/campaigns/") || | ||
| pathname.includes("/program/messages/") || | ||
| pathname.endsWith("/program/payouts/success") | ||
| ? null | ||
| : focusedProgramPath | ||
| ? isMobileDrawerViewport | ||
| ? "program" | ||
| : null | ||
| : pathname.startsWith(`/${slug}/program`) | ||
| ? "program" | ||
| : "default"; | ||
| }, [slug, pathname]); | ||
| }, [isMobile, pathname, slug, width]); | ||
|
|
||
| const { program } = useProgram({ | ||
| enabled: Boolean(currentArea === "program" && defaultProgramId), | ||
|
|
||
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.
Minor layout shift on desktop:
isRightPanelOpendefaults tofalse.On wide screens (≥960px), the right panel will be closed on the initial render, then opened by the effect on lines 104–107 after paint. This may cause a brief visible layout shift. Previously this defaulted to
true.Consider initializing based on width if available synchronously, e.g.:
Though this depends on whether
useMediaQueryprovides a synchronous initial value or not.🤖 Prompt for AI Agents