From 9028a644782d89aa6c4e82d827e2acb911fc378a Mon Sep 17 00:00:00 2001 From: Michael Matloka Date: Thu, 11 Jun 2026 18:37:45 +0200 Subject: [PATCH] fix(inbox): refetch + virtualize + server-search the people pickers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The inbox scope ("Entire project") selector kept its people query mounted permanently, so it never re-fetched on open and went stale until the 60s background refetch — the reported "assignee listing doesn't re-fetch" bug. Rework both people pickers (Reviewers + inbox scope) onto a shared, virtualized `PeoplePickerList`: - The list now lives in a Popover whose content only mounts while open, so the query re-fetches on every open. - Search is server-side via the `available_reviewers` `?query=` param (shared `useReviewerPickerOptions` hook), debounced. - Rows are virtualized so large orgs stay responsive. - An empty result surfaces a hint to connect a GitHub profile, linking to the PostHog personal settings page. Generated-By: PostHog Code Task-Id: 0bc8b5f0-0480-4c5e-8791-1a4a9736a6e7 --- .../inbox/components/InboxScopeSelect.tsx | 302 +++++++++--------- .../inbox/components/PeoplePickerList.tsx | 160 ++++++++++ .../components/SuggestedReviewersSection.tsx | 163 ++++------ .../inbox/hooks/useReviewerPickerOptions.ts | 73 +++++ 4 files changed, 448 insertions(+), 250 deletions(-) create mode 100644 packages/ui/src/features/inbox/components/PeoplePickerList.tsx create mode 100644 packages/ui/src/features/inbox/hooks/useReviewerPickerOptions.ts diff --git a/packages/ui/src/features/inbox/components/InboxScopeSelect.tsx b/packages/ui/src/features/inbox/components/InboxScopeSelect.tsx index 56aeaf71b..138a32a2a 100644 --- a/packages/ui/src/features/inbox/components/InboxScopeSelect.tsx +++ b/packages/ui/src/features/inbox/components/InboxScopeSelect.tsx @@ -1,56 +1,63 @@ -import { AsteriskSimpleIcon, CaretDownIcon } from "@phosphor-icons/react"; +import { + AsteriskSimpleIcon, + CaretDownIcon, + CheckIcon, +} from "@phosphor-icons/react"; import { INBOX_SCOPE_ENTIRE_PROJECT, INBOX_SCOPE_FOR_YOU, - type InboxScope, - isTeammateInboxScope, parseTeammateInboxScope, teammateInboxScope, } from "@posthog/core/inbox/reportMembership"; -import { - Combobox, - ComboboxContent, - ComboboxEmpty, - ComboboxInput, - ComboboxItem, - ComboboxList, -} from "@posthog/quill"; +import { PeoplePickerList } from "@posthog/ui/features/inbox/components/PeoplePickerList"; import { ReviewerAvatar } from "@posthog/ui/features/inbox/components/ReviewerAvatar"; -import { getSuggestedReviewerDisplayName } from "@posthog/ui/features/inbox/filterOptions"; +import { + getSuggestedReviewerDisplayName, + type SuggestedReviewerFilterOption, +} from "@posthog/ui/features/inbox/filterOptions"; import { useInboxScopeOptions } from "@posthog/ui/features/inbox/hooks/useInboxScopeOptions"; +import { useReviewerPickerOptions } from "@posthog/ui/features/inbox/hooks/useReviewerPickerOptions"; import { useInboxReviewerScopeStore } from "@posthog/ui/features/inbox/stores/inboxReviewerScopeStore"; -import { SegmentedControl } from "@radix-ui/themes"; -import { useMemo, useRef, useState } from "react"; +import { useDebounce } from "@posthog/ui/primitives/hooks/useDebounce"; +import { Popover, SegmentedControl } from "@radix-ui/themes"; +import { useMemo, useState } from "react"; /** * Two-segment scope toggle. Left segment is "For you"; right segment shows - * either "Entire project" or the currently-selected teammate's name, and - * opens a Quill Combobox with a searchable list of "Entire project + each - * teammate" when clicked. + * either "Entire project" or the currently-selected teammate's name, and opens + * a searchable, virtualized list of "Entire project + each teammate" when + * clicked. + * + * The list lives inside a Popover whose content only mounts while open, so the + * underlying people query re-fetches on every open (it previously stayed + * mounted and went stale until the 60s background refetch). Search is + * server-side; an empty result surfaces a hint to connect a GitHub profile. * * Segments share equal width – Radix Themes' SegmentedControl indicator is * hardcoded to equal-width math (`width: calc(100% / N)` + percentage * translate), so a fit-content override desyncs the pill from the items. * Keeping the default avoids a custom toggle just for this surface. */ -const PICKER_ENTIRE_PROJECT_VALUE = "__entire-project__"; type SegmentValue = "for-you" | "entire-project"; export function InboxScopeSelect() { const scope = useInboxReviewerScopeStore((s) => s.scope); const setScope = useInboxReviewerScopeStore((s) => s.setScope); - const anchorRef = useRef(null); const [open, setOpen] = useState(false); + // Base (always-on) list, used to resolve the selected teammate's name for the + // segment label even while the dropdown is closed. const { teammateOptions } = useInboxScopeOptions(); + const selectedTeammateUuid = parseTeammateInboxScope(scope); + const selectedTeammate = useMemo(() => { - const teammateUuid = parseTeammateInboxScope(scope); - if (!teammateUuid) return null; + if (!selectedTeammateUuid) return null; return ( - teammateOptions.find((option) => option.uuid === teammateUuid) ?? null + teammateOptions.find((option) => option.uuid === selectedTeammateUuid) ?? + null ); - }, [scope, teammateOptions]); + }, [selectedTeammateUuid, teammateOptions]); const segmentValue: SegmentValue = scope === INBOX_SCOPE_FOR_YOU ? "for-you" : "entire-project"; @@ -59,18 +66,6 @@ export function InboxScopeSelect() { ? getSuggestedReviewerDisplayName(selectedTeammate) : "Entire project"; - const pickerItems = useMemo(() => { - const items: string[] = [PICKER_ENTIRE_PROJECT_VALUE]; - for (const teammate of teammateOptions) { - items.push(teammateInboxScope(teammate.uuid)); - } - return items; - }, [teammateOptions]); - - const pickerValue: string = isTeammateInboxScope(scope) - ? scope - : PICKER_ENTIRE_PROJECT_VALUE; - const handleSegmentValueChange = (next: string) => { if (next === "for-you") { setScope(INBOX_SCOPE_FOR_YOU); @@ -78,124 +73,143 @@ export function InboxScopeSelect() { } }; - const handlePickerValueChange = (value: unknown) => { - if (typeof value !== "string") return; - if (value === PICKER_ENTIRE_PROJECT_VALUE) { - setScope(INBOX_SCOPE_ENTIRE_PROJECT); - } else { - setScope(value as InboxScope); - } + const selectEntireProject = () => { + setScope(INBOX_SCOPE_ENTIRE_PROJECT); + setOpen(false); + }; + + const selectTeammate = (teammate: SuggestedReviewerFilterOption) => { + setScope(teammateInboxScope(teammate.uuid)); setOpen(false); }; return ( - -
- - For you - setOpen(true)} - aria-haspopup="listbox" - aria-expanded={open} + + +
+ - - {rightLabel} - - - - -
- + For you + + setOpen(true)} + aria-haspopup="listbox" + aria-expanded={open} + > + + {rightLabel} + + + + +
+ + - - No matching people. - - {(itemValue: string) => { - if (itemValue === PICKER_ENTIRE_PROJECT_VALUE) { - return ( - - - - - - Entire project - - - ); - } - const teammateUuid = parseTeammateInboxScope( - itemValue as InboxScope, - ); - if (!teammateUuid) return null; - const teammate = teammateOptions.find( - (t) => t.uuid === teammateUuid, - ); - if (!teammate) return null; - const displayName = getSuggestedReviewerDisplayName(teammate); - const searchText = [ - displayName, - teammate.name, - teammate.email, - teammate.github_login, - ] - .filter(Boolean) - .join(" "); - return ( - - - - {displayName} - - - ); - }} - - - + + + ); +} + +function ScopePickerContent({ + selectedTeammateUuid, + isEntireProjectSelected, + onSelectEntireProject, + onSelectTeammate, +}: { + selectedTeammateUuid: string | null; + isEntireProjectSelected: boolean; + onSelectEntireProject: () => void; + onSelectTeammate: (teammate: SuggestedReviewerFilterOption) => void; +}) { + const [search, setSearch] = useState(""); + const debouncedSearch = useDebounce(search, 200); + const { options, isFetching } = useReviewerPickerOptions({ + query: debouncedSearch, + enabled: true, + }); + + // "For you" already covers the current user, so the teammate list excludes + // them. + const teammates = useMemo( + () => options.filter((option) => !option.isMe), + [options], + ); + + return ( + + + + + Entire project + + {isEntireProjectSelected ? ( + + ) : null} + + + } + renderRow={(teammate) => { + const displayName = getSuggestedReviewerDisplayName(teammate); + const selected = teammate.uuid === selectedTeammateUuid; + return ( + + ); + }} + /> ); } diff --git a/packages/ui/src/features/inbox/components/PeoplePickerList.tsx b/packages/ui/src/features/inbox/components/PeoplePickerList.tsx new file mode 100644 index 000000000..8faa10b2e --- /dev/null +++ b/packages/ui/src/features/inbox/components/PeoplePickerList.tsx @@ -0,0 +1,160 @@ +import { MagnifyingGlassIcon } from "@phosphor-icons/react"; +import { buildPostHogUrl } from "@posthog/core/settings/posthogUrl"; +import { useAuthStateValue } from "@posthog/ui/features/auth/store"; +import type { SuggestedReviewerFilterOption } from "@posthog/ui/features/inbox/filterOptions"; +import { openExternalUrl } from "@posthog/ui/shell/openExternal"; +import { Flex, Spinner, Text } from "@radix-ui/themes"; +import { useVirtualizer } from "@tanstack/react-virtual"; +import { type ReactNode, useRef } from "react"; + +const ESTIMATED_ROW_HEIGHT = 44; +const OVERSCAN = 8; + +interface PeoplePickerListProps { + searchQuery: string; + onSearchQueryChange: (next: string) => void; + searchPlaceholder?: string; + /** People to render in the (virtualized) list. */ + people: SuggestedReviewerFilterOption[]; + isFetching: boolean; + renderRow: ( + person: SuggestedReviewerFilterOption, + index: number, + ) => ReactNode; + /** + * Optional non-person row pinned above the searchable list (e.g. an "Entire + * project" option). Hidden while the user is searching. + */ + leadingSlot?: ReactNode; + autoFocus?: boolean; +} + +/** + * Searchable, virtualized people list shared by the inbox reviewer and scope + * pickers. Only the visible rows are mounted, so it stays responsive for large + * orgs. Search is driven by the caller (server-side); when there are no + * results it surfaces the "ask them to connect GitHub" hint. + */ +export function PeoplePickerList({ + searchQuery, + onSearchQueryChange, + searchPlaceholder = "Search people…", + people, + isFetching, + renderRow, + leadingSlot, + autoFocus, +}: PeoplePickerListProps) { + const scrollRef = useRef(null); + const isSearching = searchQuery.trim().length > 0; + + const virtualizer = useVirtualizer({ + count: people.length, + getScrollElement: () => scrollRef.current, + estimateSize: () => ESTIMATED_ROW_HEIGHT, + overscan: OVERSCAN, + getItemKey: (index) => people[index]?.uuid ?? index, + }); + + const virtualItems = virtualizer.getVirtualItems(); + const isEmpty = people.length === 0; + + return ( + + + + onSearchQueryChange(event.target.value)} + className="min-w-0 flex-1 bg-transparent text-[12px] text-gray-12 outline-none placeholder:text-(--gray-9)" + /> + {isFetching && !isEmpty ? : null} + + + {leadingSlot && !isSearching ? leadingSlot : null} + + {isEmpty ? ( + isFetching ? ( + + + + ) : ( + + ) + ) : ( +
+
+ {virtualItems.map((virtualItem) => { + const person = people[virtualItem.index]; + if (!person) return null; + return ( +
+ {renderRow(person, virtualItem.index)} +
+ ); + })} +
+
+ )} + + ); +} + +function PeoplePickerEmptyState() { + const cloudRegion = useAuthStateValue((state) => state.cloudRegion); + const integrationsUrl = buildPostHogUrl("/settings/user", cloudRegion); + + return ( +
+ + Someone not showing up here? + + + Ask them to{" "} + {integrationsUrl ? ( + + ) : ( + "connect their GitHub profile with PostHog" + )} + . + +
+ ); +} diff --git a/packages/ui/src/features/inbox/components/SuggestedReviewersSection.tsx b/packages/ui/src/features/inbox/components/SuggestedReviewersSection.tsx index faedfda49..9da4dd0df 100644 --- a/packages/ui/src/features/inbox/components/SuggestedReviewersSection.tsx +++ b/packages/ui/src/features/inbox/components/SuggestedReviewersSection.tsx @@ -1,10 +1,4 @@ -import { - CheckIcon, - MagnifyingGlassIcon, - PlusIcon, - User, - XIcon, -} from "@phosphor-icons/react"; +import { CheckIcon, PlusIcon, User, XIcon } from "@phosphor-icons/react"; import type { AvailableSuggestedReviewer, SignalReport, @@ -14,21 +8,23 @@ import type { } from "@posthog/shared/types"; import { useOptionalAuthenticatedClient } from "@posthog/ui/features/auth/authClient"; import { useCurrentUser } from "@posthog/ui/features/auth/useCurrentUser"; +import { PeoplePickerList } from "@posthog/ui/features/inbox/components/PeoplePickerList"; import { RightColumnSection } from "@posthog/ui/features/inbox/components/RightColumnSection"; import { MeBadge } from "@posthog/ui/features/inbox/components/utils/MeBadge"; import { SuggestedReviewerAvatar } from "@posthog/ui/features/inbox/components/utils/SuggestedReviewerAvatar"; import { - buildSuggestedReviewerFilterOptions, getSuggestedReviewerDisplayName, + type SuggestedReviewerFilterOption, } from "@posthog/ui/features/inbox/filterOptions"; import { - useInboxAvailableSuggestedReviewers, useInboxReportArtefacts, useUpdateSuggestedReviewers, } from "@posthog/ui/features/inbox/hooks/useInboxReports"; import { useReportActionTracker } from "@posthog/ui/features/inbox/hooks/useReportActionTracker"; +import { useReviewerPickerOptions } from "@posthog/ui/features/inbox/hooks/useReviewerPickerOptions"; +import { useDebounce } from "@posthog/ui/primitives/hooks/useDebounce"; import { Flex, Popover, Spinner, Text } from "@radix-ui/themes"; -import { useDeferredValue, useMemo, useState } from "react"; +import { useMemo, useState } from "react"; function reviewerMatchesAvailable( reviewer: SuggestedReviewer, @@ -85,7 +81,9 @@ function SuggestedReviewersBody({ const [addOpen, setAddOpen] = useState(false); const [reviewerQuery, setReviewerQuery] = useState(""); - const deferredQuery = useDeferredValue(reviewerQuery); + // Collapse the debounce window to 0 while the popover is closed so the next + // open starts from a clean query instead of replaying the last keystroke. + const debouncedQuery = useDebounce(reviewerQuery, addOpen ? 200 : 0); const { mutate: updateReviewers, isPending } = useUpdateSuggestedReviewers( report.id, @@ -100,25 +98,10 @@ function SuggestedReviewersBody({ return [reviewers[meIndex], ...reviewers.filter((_, i) => i !== meIndex)]; }, [reviewers, meUuid]); - const { data: availableReviewers, isFetching } = - useInboxAvailableSuggestedReviewers({ - enabled: !!client && addOpen, - }); - - const addableOptions = useMemo(() => { - const options = buildSuggestedReviewerFilterOptions( - availableReviewers?.results ?? [], - currentUser, - ); - const q = deferredQuery.trim().toLowerCase(); - if (!q) return options; - return options.filter( - (option) => - option.name.toLowerCase().includes(q) || - option.email.toLowerCase().includes(q) || - option.github_login.toLowerCase().includes(q), - ); - }, [availableReviewers?.results, currentUser, deferredQuery]); + const { options: addableOptions, isFetching } = useReviewerPickerOptions({ + query: debouncedQuery, + enabled: !!client && addOpen, + }); const removeReviewer = (target: SuggestedReviewer) => { const next = reviewers.filter((r) => r !== target); @@ -185,7 +168,6 @@ function SuggestedReviewersBody({ reviewers.some((r) => reviewerMatchesAvailable(r, option)) } onToggle={toggleReviewer} - hasResults={!!availableReviewers?.results?.length} /> } @@ -322,18 +304,16 @@ function AddReviewerPopover({ isPending, isAssigned, onToggle, - hasResults, }: { open: boolean; onOpenChange: (next: boolean) => void; query: string; onQueryChange: (next: string) => void; isFetching: boolean; - options: ReturnType; + options: SuggestedReviewerFilterOption[]; isPending: boolean; isAssigned: (option: AvailableSuggestedReviewer) => boolean; onToggle: (option: AvailableSuggestedReviewer) => void; - hasResults: boolean; }) { return ( @@ -353,78 +333,49 @@ function AddReviewerPopover({ sideOffset={6} className="min-w-[280px] max-w-[320px] p-2" > - - - - onQueryChange(e.target.value)} - className="min-w-0 flex-1 bg-transparent text-[12px] text-gray-12 outline-none placeholder:text-(--gray-9)" - /> - -
- {isFetching && !hasResults ? ( - - - - ) : options.length === 0 ? ( - - No users found. - - ) : ( - - {options.map((option) => { - const assigned = isAssigned(option); - const displayName = getSuggestedReviewerDisplayName(option); - return ( - - ); - })} - - )} -
- + { + const assigned = isAssigned(option); + const displayName = getSuggestedReviewerDisplayName(option); + return ( + + ); + }} + /> ); diff --git a/packages/ui/src/features/inbox/hooks/useReviewerPickerOptions.ts b/packages/ui/src/features/inbox/hooks/useReviewerPickerOptions.ts new file mode 100644 index 000000000..fb15a10be --- /dev/null +++ b/packages/ui/src/features/inbox/hooks/useReviewerPickerOptions.ts @@ -0,0 +1,73 @@ +import { useOptionalAuthenticatedClient } from "@posthog/ui/features/auth/authClient"; +import { useCurrentUser } from "@posthog/ui/features/auth/useCurrentUser"; +import { + buildSuggestedReviewerFilterOptions, + type SuggestedReviewerFilterOption, +} from "@posthog/ui/features/inbox/filterOptions"; +import { useInboxAvailableSuggestedReviewers } from "@posthog/ui/features/inbox/hooks/useInboxReports"; +import { useMemo } from "react"; + +interface UseReviewerPickerOptionsParams { + /** + * Server-side search term. An empty string fetches the full base list; any + * other value is forwarded to the `available_reviewers` endpoint, which does + * the matching server-side. + */ + query?: string; + /** + * Gate the underlying fetch. Pass the picker's open state so the list + * re-fetches every time it is opened (the query mounts fresh, and + * `refetchOnMount: "always"` then pulls the latest people). + */ + enabled?: boolean; +} + +export interface ReviewerPickerOptions { + options: SuggestedReviewerFilterOption[]; + isFetching: boolean; + hasResults: boolean; +} + +/** + * Shared data source for the people pickers (suggested reviewers + inbox scope + * teammate selector). Centralises server-side search and the "pin me to the + * top" behaviour so both surfaces stay consistent. + */ +export function useReviewerPickerOptions( + params?: UseReviewerPickerOptionsParams, +): ReviewerPickerOptions { + const client = useOptionalAuthenticatedClient(); + const { data: currentUser } = useCurrentUser({ client }); + const query = params?.query?.trim() ?? ""; + const isSearching = query.length > 0; + + const { data, isFetching } = useInboxAvailableSuggestedReviewers({ + query, + enabled: params?.enabled, + }); + + const options = useMemo( + () => + buildSuggestedReviewerFilterOptions( + data?.results ?? [], + // Only pin the current user onto the unfiltered base list. While the + // user is searching the server already decides who matches, so forcing + // "me" in would surface the current user for unrelated queries. + isSearching || !currentUser + ? null + : { + uuid: currentUser.uuid, + email: currentUser.email, + first_name: currentUser.first_name, + last_name: currentUser.last_name, + }, + ), + [data?.results, currentUser, isSearching], + ); + + return { + options, + isFetching, + hasResults: !!data?.results?.length, + }; +}