-
Notifications
You must be signed in to change notification settings - Fork 67
feat: add recent activity hover popover #3874
Changes from all commits
77b2b04
76d85d7
42331e0
bfc6e1f
b5a98a5
0f5ec39
7e5c21a
3137fae
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 |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| import { render, waitFor } from "@testing-library/react"; | ||
| import type { ReactNode } from "react"; | ||
| import { beforeEach, describe, expect, it, vi } from "vitest"; | ||
|
|
||
| const mocks = vi.hoisted(() => ({ | ||
| fetchNextPage: vi.fn(), | ||
| hasNextPage: true, | ||
| isFetchingNextPage: false, | ||
| })); | ||
|
|
||
| vi.mock("@posthog/quill", () => ({ | ||
| Button: ({ children }: { children: ReactNode }) => ( | ||
| <button type="button">{children}</button> | ||
| ), | ||
| Empty: ({ children }: { children: ReactNode }) => <div>{children}</div>, | ||
| EmptyDescription: ({ children }: { children: ReactNode }) => ( | ||
| <div>{children}</div> | ||
| ), | ||
| EmptyHeader: ({ children }: { children: ReactNode }) => <div>{children}</div>, | ||
| EmptyMedia: ({ children }: { children: ReactNode }) => <div>{children}</div>, | ||
| EmptyTitle: ({ children }: { children: ReactNode }) => <div>{children}</div>, | ||
| PopoverContent: ({ children }: { children: ReactNode }) => ( | ||
| <div>{children}</div> | ||
| ), | ||
| Spinner: () => <div>Loading</div>, | ||
| })); | ||
| vi.mock("@posthog/ui/features/auth/authClient", () => ({ | ||
| useOptionalAuthenticatedClient: () => ({}), | ||
| })); | ||
| vi.mock("@posthog/ui/features/auth/useCurrentUser", () => ({ | ||
| useCurrentUser: () => ({ data: null }), | ||
| })); | ||
| vi.mock("@posthog/ui/features/canvas/components/ActivityView", () => ({ | ||
| ActivityRow: () => <div>Activity row</div>, | ||
| })); | ||
| vi.mock("@posthog/ui/features/canvas/hooks/useChannels", () => ({ | ||
| useChannels: () => ({ channels: [] }), | ||
| })); | ||
| vi.mock("@posthog/ui/features/canvas/hooks/useMarkTaskActivityRead", () => ({ | ||
| useMarkTaskActivityRead: () => ({ mutate: vi.fn(), isPending: false }), | ||
| })); | ||
| vi.mock("@posthog/ui/features/canvas/hooks/useTaskActivity", () => ({ | ||
| useTaskActivity: () => ({ | ||
| items: [], | ||
| unreadCount: 0, | ||
| isLoading: false, | ||
| hasNextPage: mocks.hasNextPage, | ||
| isFetchingNextPage: mocks.isFetchingNextPage, | ||
| fetchNextPage: mocks.fetchNextPage, | ||
| }), | ||
| })); | ||
| vi.mock("@posthog/ui/primitives/hooks/useInView", () => ({ | ||
| useInView: () => [vi.fn(), true], | ||
| })); | ||
| vi.mock("@posthog/ui/shell/analytics", () => ({ track: vi.fn() })); | ||
|
|
||
| import { ActivityHoverCard } from "./ActivityHoverCard"; | ||
|
|
||
| describe("ActivityHoverCard", () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| mocks.hasNextPage = true; | ||
| mocks.isFetchingNextPage = false; | ||
| }); | ||
|
|
||
| it("loads the next page when the bottom sentinel is visible", async () => { | ||
| render(<ActivityHoverCard onClose={vi.fn()} />); | ||
|
|
||
| await waitFor(() => expect(mocks.fetchNextPage).toHaveBeenCalledOnce()); | ||
| }); | ||
|
|
||
| it("does not load when there is no next page", async () => { | ||
| mocks.hasNextPage = false; | ||
| render(<ActivityHoverCard onClose={vi.fn()} />); | ||
|
|
||
| await waitFor(() => expect(mocks.fetchNextPage).not.toHaveBeenCalled()); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,151 @@ | ||
| import { BellIcon, ChecksIcon } from "@phosphor-icons/react"; | ||
| import { | ||
| Button, | ||
| Empty, | ||
| EmptyDescription, | ||
| EmptyHeader, | ||
| EmptyMedia, | ||
| EmptyTitle, | ||
| PopoverContent, | ||
| Spinner, | ||
| } from "@posthog/quill"; | ||
| import { ANALYTICS_EVENTS } from "@posthog/shared/analytics-events"; | ||
| import { useOptionalAuthenticatedClient } from "@posthog/ui/features/auth/authClient"; | ||
| import { useCurrentUser } from "@posthog/ui/features/auth/useCurrentUser"; | ||
| import { ActivityRow } from "@posthog/ui/features/canvas/components/ActivityView"; | ||
| import { useChannels } from "@posthog/ui/features/canvas/hooks/useChannels"; | ||
| import { useMarkTaskActivityRead } from "@posthog/ui/features/canvas/hooks/useMarkTaskActivityRead"; | ||
| import { useTaskActivity } from "@posthog/ui/features/canvas/hooks/useTaskActivity"; | ||
| import { useInView } from "@posthog/ui/primitives/hooks/useInView"; | ||
| import { track } from "@posthog/ui/shell/analytics"; | ||
| import { useEffect, useMemo, useState } from "react"; | ||
| import { | ||
| activityReadPayload, | ||
| channelIdForName, | ||
| createChannelIdByName, | ||
| getUnreadActivityItems, | ||
| markLoadedReadLabel, | ||
| } from "./activityFeed"; | ||
|
|
||
| interface ActivityHoverCardProps { | ||
| onClose: () => void; | ||
| side?: "bottom" | "right"; | ||
| } | ||
|
|
||
| export function ActivityHoverCard({ | ||
| onClose, | ||
| side = "right", | ||
| }: ActivityHoverCardProps) { | ||
| const client = useOptionalAuthenticatedClient(); | ||
| const { data: currentUser } = useCurrentUser({ client }); | ||
| const { | ||
| items, | ||
| unreadCount, | ||
| isLoading, | ||
| hasNextPage, | ||
| isFetchingNextPage, | ||
| fetchNextPage, | ||
| } = useTaskActivity(); | ||
| const [scrollRoot, setScrollRoot] = useState<HTMLDivElement | null>(null); | ||
| const [loadMoreRef, loadMoreInView] = useInView<HTMLDivElement>({ | ||
| root: scrollRoot, | ||
| rootMargin: "100px 0px", | ||
| }); | ||
| const unreadItems = getUnreadActivityItems(items); | ||
| const { mutate: markTasksRead, isPending: isMarkingRead } = | ||
| useMarkTaskActivityRead(); | ||
| const { channels } = useChannels(); | ||
| const folderIdByName = useMemo( | ||
| () => createChannelIdByName(channels), | ||
| [channels], | ||
| ); | ||
| useEffect(() => { | ||
| track(ANALYTICS_EVENTS.CHANNEL_ACTION, { | ||
| action_type: "view_activity", | ||
| surface: "activity_panel", | ||
| }); | ||
| }, []); | ||
| useEffect(() => { | ||
|
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.
I couldn't confirm the server page size — Separately:
Contributor
Author
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. Fixed: pagination observes the popover scroll container and no longer retriggers when a request settles. |
||
| if (loadMoreInView && hasNextPage) { | ||
| void fetchNextPage(); | ||
| } | ||
| }, [fetchNextPage, hasNextPage, loadMoreInView]); | ||
|
|
||
| const markRead = (taskId: string, activityAt: string) => { | ||
| markTasksRead([{ task_id: taskId, seen_before: activityAt }]); | ||
| }; | ||
|
|
||
| const markAllRead = () => { | ||
| markTasksRead(activityReadPayload(unreadItems)); | ||
| }; | ||
|
|
||
| return ( | ||
| <PopoverContent | ||
| side={side} | ||
| align="start" | ||
| sideOffset={8} | ||
| className="w-[380px] gap-0 overflow-hidden p-0" | ||
| > | ||
| <div className="flex min-h-12 items-center justify-between border-border border-b px-3"> | ||
| <span className="font-semibold text-sm">Activity</span> | ||
| {unreadItems.length > 0 && ( | ||
| <Button | ||
| variant="default" | ||
| size="sm" | ||
| loading={isMarkingRead} | ||
| disabled={isMarkingRead} | ||
| onClick={markAllRead} | ||
| > | ||
| <ChecksIcon size={14} /> | ||
| {markLoadedReadLabel(unreadItems.length, unreadCount)} | ||
| </Button> | ||
| )} | ||
| </div> | ||
| <div ref={setScrollRoot} className="max-h-[480px] overflow-y-auto p-1.5"> | ||
| {isLoading && items.length === 0 ? ( | ||
| <div className="flex justify-center py-10"> | ||
| <Spinner /> | ||
| </div> | ||
| ) : items.length === 0 ? ( | ||
| <Empty className="border-0 py-8"> | ||
| <EmptyHeader> | ||
| <EmptyMedia variant="icon"> | ||
| <BellIcon /> | ||
| </EmptyMedia> | ||
| <EmptyTitle>No recent activity</EmptyTitle> | ||
| <EmptyDescription> | ||
| New task updates will appear here. | ||
| </EmptyDescription> | ||
| </EmptyHeader> | ||
| </Empty> | ||
| ) : ( | ||
| <div className="flex flex-col gap-0.5"> | ||
| {items.map((item) => ( | ||
| <ActivityRow | ||
| key={item.taskId} | ||
| item={item} | ||
| folderChannelId={channelIdForName( | ||
| folderIdByName, | ||
| item.channelName, | ||
| )} | ||
| onOpen={(activity) => | ||
| markRead(activity.taskId, activity.activityAt) | ||
| } | ||
| onMarkRead={(activity) => | ||
| markRead(activity.taskId, activity.activityAt) | ||
| } | ||
| currentUser={currentUser} | ||
| surface="activity_panel" | ||
| onNavigate={onClose} | ||
| compact | ||
| /> | ||
| ))} | ||
| </div> | ||
| )} | ||
| <div ref={loadMoreRef} className="flex h-8 justify-center py-2"> | ||
| {hasNextPage && isFetchingNextPage && <Spinner />} | ||
| </div> | ||
| </div> | ||
| </PopoverContent> | ||
| ); | ||
| } | ||
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.
The whole activity-feed shell is duplicated from
ActivityView, and the two copies have already diverged.Lines 16-61 mirror
ActivityView.tsx:258-296: the sameuseTaskActivity/useMarkTaskActivityRead/useChannelsdestructuring, an identicalnew Map(channels.map(c => [normalizeChannelName(c.name), c.id])), hand-built{ task_id, seen_before }payloads, and the same mount-effecttrack(CHANNEL_ACTION, { action_type: "view_activity" }).They already differ in a way that's a live bug: this popover relabels its button "Mark visible as read" when loaded unread rows don't cover
unreadCount, while the page unconditionally says "Mark all as read" from the same partially-loadeditems— so the page's label is now wrong and there's no single place to fix it.The empty state here is also a hand-rolled div where
ActivityViewuses quill's<Empty>, which AGENTS.md:235 requires: "Empty/placeholder/loading screens (canvas and elsewhere) are a@posthog/quill<Empty>… Don't hand-roll the centered Flex + dashed icon box." Same PR, two different empty states for the same data.Extracting a shared
useActivityFeed(surface)+ActivityListwould remove ~35 duplicated lines and leave each surface only its chrome.Uh oh!
There was an error while loading. Please reload this page.
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.
Fixed: both surfaces now share unread payload, label, and channel-lookup helpers; the popover also uses Quill Empty components.