Skip to content
This repository was archived by the owner on Aug 6, 2026. It is now read-only.

Commit 87d00c3

Browse files
authored
fix(canvas): never refuse a hover-card open on the Activity bell
The first pass swallowed the open that quill's trigger applies on click, which stopped the stray card but left the underlying problem: refusing an open desyncs the trigger. Base UI applies the open internally before we see it, so handing back `false` leaves the bell stuck with `data-popup-open`/`data-pressed` and its hover-open dead until the trigger remounts. The old `!isActivity` gate refused opens the same way, so this was reachable on the Activity page too. Remove the refusals instead: the trigger's own click-open is prevented outright with `preventBaseUIHandler`, and the Activity page renders the bell with no popover at all. Owning the open state in a component that only mounts off the Activity page means it is born closed on every visit, so there is nothing to mask and nothing left over to resurface. Generated-By: PostHog Code Task-Id: f89ba875-c0c8-4a08-b088-67b2a900e66d
1 parent 14d9ed7 commit 87d00c3

2 files changed

Lines changed: 83 additions & 58 deletions

File tree

‎packages/ui/src/features/canvas/components/ChannelNav.test.tsx‎

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,26 +74,51 @@ describe("ChannelNav", () => {
7474

7575
const activity = screen.getByLabelText("Activity");
7676
expect(activity).toBeEnabled();
77+
expect(activity).not.toHaveAttribute("aria-haspopup");
7778
await user.hover(activity);
7879

7980
await new Promise((resolve) => setTimeout(resolve, 400));
8081
expect(screen.queryByText("Recent activity card")).not.toBeInTheDocument();
8182
});
8283

83-
it("does not resurface the hover card after the bell navigated to Activity", async () => {
84+
it("leaves no popover state on the bell after it navigates to Activity", async () => {
8485
const user = userEvent.setup();
8586
const { rerender } = render(<ChannelNav />);
87+
const bell = () => screen.getByLabelText("Activity");
8688

87-
await user.click(screen.getByLabelText("Activity"));
89+
await user.hover(bell());
90+
await screen.findByText("Recent activity card", {}, { timeout: 1_000 });
91+
await user.click(bell());
8892
mocks.view = { type: "activity" };
8993
rerender(<ChannelNav />);
94+
9095
expect(screen.queryByText("Recent activity card")).not.toBeInTheDocument();
96+
expect(bell()).not.toHaveAttribute("data-popup-open");
97+
expect(bell()).not.toHaveAttribute("data-pressed");
98+
});
9199

92-
// Opening a notification from the Activity page navigates to its task.
93-
mocks.view = { type: "task-detail" };
100+
it("neither resurfaces nor wedges the hover card once the bell has navigated", async () => {
101+
const user = userEvent.setup();
102+
const { rerender } = render(<ChannelNav />);
103+
const bell = () => screen.getByLabelText("Activity");
104+
105+
await user.hover(bell());
106+
await user.click(bell());
107+
mocks.view = { type: "activity" };
94108
rerender(<ChannelNav />);
109+
await user.unhover(bell());
95110

111+
// Opening a notification from the Activity page navigates to its task: the
112+
// card must not come along for the ride.
113+
mocks.view = { type: "task-detail" };
114+
rerender(<ChannelNav />);
96115
await new Promise((resolve) => setTimeout(resolve, 400));
97116
expect(screen.queryByText("Recent activity card")).not.toBeInTheDocument();
117+
118+
// ...and hover must still work there.
119+
await user.hover(bell());
120+
expect(
121+
await screen.findByText("Recent activity card", {}, { timeout: 1_000 }),
122+
).toBeInTheDocument();
98123
});
99124
});

‎packages/ui/src/features/canvas/components/ChannelNav.tsx‎

Lines changed: 54 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ import { useAppView } from "@posthog/ui/router/useAppView";
4141
import { track } from "@posthog/ui/shell/analytics";
4242
import {
4343
type ComponentPropsWithRef,
44+
type ReactElement,
4445
type ReactNode,
45-
useRef,
4646
useState,
4747
} from "react";
4848
import { ActivityHoverCard } from "./ActivityHoverCard";
@@ -138,6 +138,54 @@ function NavButton({
138138
);
139139
}
140140

141+
// Only mounted off the Activity page, so the card's open state is born fresh on
142+
// every visit. That matters because refusing an open is not free: quill's
143+
// trigger applies it internally before we see it, so a `false` we hand back
144+
// leaves the trigger stuck in its pressed state with hover-open dead until it
145+
// remounts. Nothing here refuses one — the click prevents the trigger's own
146+
// open, and the Activity page renders the bell without a popover at all.
147+
function ActivityHoverPopover({ trigger }: { trigger: ReactElement }) {
148+
const [open, setOpen] = useState(false);
149+
150+
return (
151+
<Popover open={open} onOpenChange={setOpen}>
152+
<PopoverTrigger
153+
openOnHover
154+
delay={300}
155+
closeDelay={100}
156+
onClick={(event) => event.preventBaseUIHandler()}
157+
render={trigger}
158+
/>
159+
{open && (
160+
<ActivityHoverCard side="bottom" onClose={() => setOpen(false)} />
161+
)}
162+
</Popover>
163+
);
164+
}
165+
166+
function ActivityNavItem({
167+
isActive,
168+
unreadCount,
169+
onNavigate,
170+
}: {
171+
isActive: boolean;
172+
unreadCount: number;
173+
onNavigate: () => void;
174+
}) {
175+
const bell = (
176+
<NavButton
177+
icon={<BellIcon size={16} weight={isActive ? "fill" : "regular"} />}
178+
label="Activity"
179+
isActive={isActive}
180+
onClick={onNavigate}
181+
badge={<CountBadge count={unreadCount} className={ICON_BADGE_CLASS} />}
182+
/>
183+
);
184+
185+
if (isActive) return bell;
186+
return <ActivityHoverPopover trigger={bell} />;
187+
}
188+
141189
export function ChannelNav() {
142190
const view = useAppView();
143191
const loopsEnabled = useFeatureFlag(LOOPS_FLAG, import.meta.env.DEV);
@@ -162,15 +210,6 @@ export function ChannelNav() {
162210
const isActivity = view.type === "activity";
163211
const isCommandCenter = view.type === "command-center";
164212

165-
// Clicking the bell navigates to Activity, but quill's trigger runs its own
166-
// open after our handler and still inside the same click, so `isActivity` is
167-
// false and the card records itself as open. The Activity page then only hides
168-
// it, and it resurfaces over the next page you open. Swallowing that one open
169-
// is what the ref is for — the sidebar's Activity row does the same.
170-
const [activityOpen, setActivityOpen] = useState(false);
171-
const suppressClickOpenRef = useRef(false);
172-
const activityCardOpen = activityOpen && !isActivity;
173-
174213
return (
175214
// One provider for the row: once any tooltip is up, moving to its
176215
// neighbour reveals that one immediately instead of serving the warm-up
@@ -191,50 +230,11 @@ export function ChannelNav() {
191230
<CountBadge count={counts.pulls} className={ICON_BADGE_CLASS} />
192231
}
193232
/>
194-
<Popover
195-
open={activityCardOpen}
196-
onOpenChange={(open) => {
197-
const suppressed = open && suppressClickOpenRef.current;
198-
suppressClickOpenRef.current = false;
199-
if (suppressed) return;
200-
setActivityOpen(!isActivity && open);
201-
}}
202-
>
203-
<PopoverTrigger
204-
openOnHover
205-
delay={300}
206-
closeDelay={100}
207-
render={
208-
<NavButton
209-
icon={
210-
<BellIcon
211-
size={16}
212-
weight={isActivity ? "fill" : "regular"}
213-
/>
214-
}
215-
label="Activity"
216-
isActive={isActivity}
217-
onClick={() => {
218-
suppressClickOpenRef.current = true;
219-
setActivityOpen(false);
220-
withTrack("activity", navigateToActivity)();
221-
}}
222-
badge={
223-
<CountBadge
224-
count={unseenActivity}
225-
className={ICON_BADGE_CLASS}
226-
/>
227-
}
228-
/>
229-
}
230-
/>
231-
{activityCardOpen && (
232-
<ActivityHoverCard
233-
side="bottom"
234-
onClose={() => setActivityOpen(false)}
235-
/>
236-
)}
237-
</Popover>
233+
<ActivityNavItem
234+
isActive={isActivity}
235+
unreadCount={unseenActivity}
236+
onNavigate={withTrack("activity", navigateToActivity)}
237+
/>
238238
<NavIcon
239239
icon={
240240
<Lightning

0 commit comments

Comments
 (0)