Skip to content

Commit 131f261

Browse files
authored
fix(channels): address react-doctor and review findings on mention composer
applyMention now replaces the whole @word (caret moved backward mid-query no longer leaks typed characters into the message) and reuses an existing following space instead of doubling it. MentionComposer drops its state-sync effects for ref-guarded render-time adjustments and scrolls in the key handler, clearing react-doctor's no-adjust-state-on-prop-change errors. MentionText keys segments by character offset instead of array index. Generated-By: PostHog Code Task-Id: a2ff8500-aaa6-4ce0-b01a-656d3b2c3b61
1 parent 53052ef commit 131f261

4 files changed

Lines changed: 79 additions & 37 deletions

File tree

packages/ui/src/features/canvas/components/MentionComposer.tsx

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,7 @@ import {
1414
} from "@posthog/ui/features/canvas/utils/mentionComposer";
1515
import { userDisplayName } from "@posthog/ui/features/canvas/utils/userDisplay";
1616
import { Text } from "@radix-ui/themes";
17-
import {
18-
type ReactNode,
19-
useCallback,
20-
useEffect,
21-
useMemo,
22-
useRef,
23-
useState,
24-
} from "react";
17+
import { type ReactNode, useCallback, useMemo, useRef, useState } from "react";
2518

2619
interface MentionComposerProps {
2720
value: string;
@@ -73,21 +66,28 @@ export function MentionComposer({
7366
const open =
7467
!!active && active.start !== dismissedStart && suggestions.length > 0;
7568

76-
// biome-ignore lint/correctness/useExhaustiveDependencies: reset selection per query
77-
useEffect(() => {
78-
setSelectedIndex(0);
79-
}, [active?.start, active?.query]);
80-
81-
useEffect(() => {
69+
// Render-time adjustments (ref-guarded, same idiom as SuggestionList): when
70+
// the parent clears the draft the mention context goes with it, and a new
71+
// query restarts keyboard selection at the top.
72+
const prevValueRef = useRef(value);
73+
if (prevValueRef.current !== value) {
74+
prevValueRef.current = value;
8275
if (!value) {
8376
setActive(null);
8477
setDismissedStart(null);
8578
}
86-
}, [value]);
87-
88-
useEffect(() => {
89-
itemRefs.current[selectedIndex]?.scrollIntoView({ block: "nearest" });
90-
}, [selectedIndex]);
79+
}
80+
const activeKey = active ? `${active.start}:${active.query}` : "";
81+
const prevActiveKeyRef = useRef(activeKey);
82+
if (prevActiveKeyRef.current !== activeKey) {
83+
prevActiveKeyRef.current = activeKey;
84+
if (selectedIndex !== 0) setSelectedIndex(0);
85+
}
86+
// The list can shrink while a lower row is selected (members filter down).
87+
const highlightedIndex = Math.min(
88+
selectedIndex,
89+
Math.max(0, suggestions.length - 1),
90+
);
9191

9292
const insert = useCallback(
9393
(member: UserBasic) => {
@@ -111,12 +111,14 @@ export function MentionComposer({
111111
if (event.key === "ArrowDown" || event.key === "ArrowUp") {
112112
event.preventDefault();
113113
const delta = event.key === "ArrowDown" ? 1 : suggestions.length - 1;
114-
setSelectedIndex((i) => (i + delta) % suggestions.length);
114+
const next = (highlightedIndex + delta) % suggestions.length;
115+
setSelectedIndex(next);
116+
itemRefs.current[next]?.scrollIntoView({ block: "nearest" });
115117
return;
116118
}
117119
if (event.key === "Enter" || event.key === "Tab") {
118120
event.preventDefault();
119-
const member = suggestions[selectedIndex];
121+
const member = suggestions[highlightedIndex];
120122
if (member) insert(member);
121123
return;
122124
}
@@ -145,7 +147,7 @@ export function MentionComposer({
145147
<button
146148
type="button"
147149
role="option"
148-
aria-selected={index === selectedIndex}
150+
aria-selected={index === highlightedIndex}
149151
key={member.uuid}
150152
ref={(el) => {
151153
itemRefs.current[index] = el;
@@ -155,7 +157,7 @@ export function MentionComposer({
155157
onClick={() => insert(member)}
156158
onMouseEnter={() => setSelectedIndex(index)}
157159
className={`flex w-full items-center gap-2 border-none px-2 py-1 text-left ${
158-
index === selectedIndex ? "bg-[var(--accent-a4)]" : ""
160+
index === highlightedIndex ? "bg-[var(--accent-a4)]" : ""
159161
}`}
160162
>
161163
<Avatar size="xs" className="shrink-0">

packages/ui/src/features/canvas/components/MentionText.tsx

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { splitMentionSegments } from "@posthog/shared";
22
import { Text } from "@radix-ui/themes";
3-
import { useMemo } from "react";
3+
import { Fragment, useMemo } from "react";
44

55
/**
66
* Thread message content with inline mention tokens rendered as highlighted
@@ -15,15 +15,22 @@ export function MentionText({
1515
currentUserEmail?: string | null;
1616
className?: string;
1717
}) {
18-
const segments = useMemo(() => splitMentionSegments(content), [content]);
18+
// Key each segment by its character offset — stable for a given content.
19+
const segments = useMemo(() => {
20+
let offset = 0;
21+
return splitMentionSegments(content).map((segment) => {
22+
const entry = { segment, key: `${offset}` };
23+
offset += segment.text.length;
24+
return entry;
25+
});
26+
}, [content]);
1927
const selfEmail = currentUserEmail?.toLowerCase();
2028
return (
2129
<Text size="1" className={className}>
22-
{segments.map((segment, index) =>
30+
{segments.map(({ segment, key }) =>
2331
segment.type === "mention" ? (
2432
<span
25-
// biome-ignore lint/suspicious/noArrayIndexKey: segments are positional
26-
key={index}
33+
key={key}
2734
className={`rounded px-0.5 font-medium ${
2835
selfEmail && segment.email.toLowerCase() === selfEmail
2936
? "bg-[var(--accent-a4)] text-[var(--accent-12)]"
@@ -34,7 +41,7 @@ export function MentionText({
3441
@{segment.name}
3542
</span>
3643
) : (
37-
segment.text
44+
<Fragment key={key}>{segment.text}</Fragment>
3845
),
3946
)}
4047
</Text>

packages/ui/src/features/canvas/utils/mentionComposer.test.ts

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,16 +90,37 @@ describe("filterMentionCandidates", () => {
9090
});
9191

9292
describe("applyMention", () => {
93-
it("replaces the active query with a token and trailing space", () => {
93+
it("replaces the active query, reusing the existing following space", () => {
9494
const text = "hey @raq can you look";
9595
const active = { start: 4, query: "raq" };
9696
const result = applyMention(text, active, 8, raquel);
9797
expect(result.text).toBe(
98-
"hey @[Raquel Smith](raquel@posthog.com) can you look",
98+
"hey @[Raquel Smith](raquel@posthog.com) can you look",
9999
);
100-
expect(result.caret).toBe(
101-
"hey @[Raquel Smith](raquel@posthog.com) ".length,
100+
expect(result.caret).toBe("hey @[Raquel Smith](raquel@posthog.com) ".length);
101+
});
102+
103+
it("consumes the rest of the @word when the caret moved backward", () => {
104+
// "hey @raq" with the caret between "ra" and "q": the trailing "q" is
105+
// still query text and must not leak into the message.
106+
const result = applyMention(
107+
"hey @raq",
108+
{ start: 4, query: "ra" },
109+
7,
110+
raquel,
111+
);
112+
expect(result.text).toBe("hey @[Raquel Smith](raquel@posthog.com) ");
113+
expect(result.caret).toBe(result.text.length);
114+
});
115+
116+
it("does not consume text beyond the @word", () => {
117+
const result = applyMention(
118+
"@ra world",
119+
{ start: 0, query: "ra" },
120+
3,
121+
raquel,
102122
);
123+
expect(result.text).toBe("@[Raquel Smith](raquel@posthog.com) world");
103124
});
104125

105126
it("works at the end of the text", () => {

packages/ui/src/features/canvas/utils/mentionComposer.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,15 +56,27 @@ export function filterMentionCandidates(
5656
.map((entry) => entry.member);
5757
}
5858

59-
/** Replace the active `@query` with the member's mention token. */
59+
/**
60+
* Replace the active `@query` with the member's mention token, leaving the
61+
* caret right after it (past any space that already follows).
62+
*/
6063
export function applyMention(
6164
text: string,
6265
active: ActiveMentionQuery,
6366
caret: number,
6467
member: UserBasic,
6568
): { text: string; caret: number } {
66-
const token = `${formatMention(userDisplayName(member), member.email)} `;
69+
// The replacement spans the whole @word: when the caret moved back inside
70+
// the query, the characters typed after it are still mention text.
71+
let end = caret;
72+
while (end < text.length && !/\s/.test(text[end] ?? "")) end++;
73+
const tail = text.slice(end);
74+
const token = formatMention(userDisplayName(member), member.email);
6775
const before = text.slice(0, active.start);
68-
const next = before + token + text.slice(caret);
69-
return { text: next, caret: before.length + token.length };
76+
// Reuse an existing following space rather than doubling it up.
77+
const inserted = tail.startsWith(" ") ? token : `${token} `;
78+
return {
79+
text: before + inserted + tail,
80+
caret: before.length + inserted.length + (tail.startsWith(" ") ? 1 : 0),
81+
};
7082
}

0 commit comments

Comments
 (0)