-
Notifications
You must be signed in to change notification settings - Fork 52
fix(queued-messages): clamp long queued messages to 3 lines #3163
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
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 | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,10 +1,12 @@ | ||||||||||||||||||||||||
| import { useLayoutEffect, useRef, useState } from "react"; | ||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||
| ArrowBendDownLeft, | ||||||||||||||||||||||||
| CaretDown, | ||||||||||||||||||||||||
| PencilSimple, | ||||||||||||||||||||||||
| Stack, | ||||||||||||||||||||||||
| Trash, | ||||||||||||||||||||||||
| } from "@phosphor-icons/react"; | ||||||||||||||||||||||||
| import { Button } from "@posthog/quill"; | ||||||||||||||||||||||||
| import { Button, cn } from "@posthog/quill"; | ||||||||||||||||||||||||
| import { Box, Flex, IconButton, Tooltip } from "@radix-ui/themes"; | ||||||||||||||||||||||||
| import { MarkdownRenderer } from "../../../editor/components/MarkdownRenderer"; | ||||||||||||||||||||||||
| import type { QueuedMessage } from "../../sessionStore"; | ||||||||||||||||||||||||
|
|
@@ -29,15 +31,56 @@ export function QueuedMessageView({ | |||||||||||||||||||||||
| ? "Inject this message into the current turn at the next tool boundary." | ||||||||||||||||||||||||
| : "Interrupt the current turn and resend with this message."; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const [isExpanded, setIsExpanded] = useState(false); | ||||||||||||||||||||||||
| const [isOverflowing, setIsOverflowing] = useState(false); | ||||||||||||||||||||||||
| const textRef = useRef<HTMLDivElement>(null); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Only meaningful while collapsed: expanding removes the clamp so scrollHeight === clientHeight. | ||||||||||||||||||||||||
| // We keep the prior result when expanded so the "Show less" trigger stays put. | ||||||||||||||||||||||||
| // biome-ignore lint/correctness/useExhaustiveDependencies: re-measure when the message text changes. | ||||||||||||||||||||||||
| useLayoutEffect(() => { | ||||||||||||||||||||||||
| if (isExpanded) return; | ||||||||||||||||||||||||
| const el = textRef.current; | ||||||||||||||||||||||||
| if (!el) return; | ||||||||||||||||||||||||
| const measure = () => | ||||||||||||||||||||||||
| setIsOverflowing(el.scrollHeight - el.clientHeight > 1); | ||||||||||||||||||||||||
| measure(); | ||||||||||||||||||||||||
| const observer = new ResizeObserver(measure); | ||||||||||||||||||||||||
| observer.observe(el); | ||||||||||||||||||||||||
| return () => observer.disconnect(); | ||||||||||||||||||||||||
| }, [message.content, isExpanded]); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||
| <Box className="rounded-lg border border-gray-5 bg-card px-3 py-2"> | ||||||||||||||||||||||||
| <Flex align="center" gap="2"> | ||||||||||||||||||||||||
| <Stack size={14} className="shrink-0 text-gray-9" /> | ||||||||||||||||||||||||
| <Flex align="start" gap="2"> | ||||||||||||||||||||||||
| <Stack size={14} className="shrink-0 text-gray-9 mt-0.5" /> | ||||||||||||||||||||||||
| <Box className="min-w-0 flex-1 font-medium text-[13px] text-gray-12 [&>*:last-child]:mb-0"> | ||||||||||||||||||||||||
| {hasFileMentions(message.content) ? ( | ||||||||||||||||||||||||
| parseFileMentions(message.content) | ||||||||||||||||||||||||
| ) : ( | ||||||||||||||||||||||||
| <MarkdownRenderer content={message.content} /> | ||||||||||||||||||||||||
| <div | ||||||||||||||||||||||||
| ref={textRef} | ||||||||||||||||||||||||
| className={cn( | ||||||||||||||||||||||||
| !isExpanded && "max-h-[3lh] overflow-hidden", | ||||||||||||||||||||||||
| !isExpanded && | ||||||||||||||||||||||||
| isOverflowing && | ||||||||||||||||||||||||
| "[mask-image:linear-gradient(to_bottom,black_45%,transparent)]", | ||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||
| > | ||||||||||||||||||||||||
| {hasFileMentions(message.content) ? ( | ||||||||||||||||||||||||
| parseFileMentions(message.content) | ||||||||||||||||||||||||
| ) : ( | ||||||||||||||||||||||||
| <MarkdownRenderer content={message.content} /> | ||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||
| {isOverflowing && ( | ||||||||||||||||||||||||
| <button | ||||||||||||||||||||||||
| type="button" | ||||||||||||||||||||||||
| onClick={() => setIsExpanded((v) => !v)} | ||||||||||||||||||||||||
| className="mt-1 flex items-center gap-0.5 text-muted-foreground text-xs hover:text-foreground" | ||||||||||||||||||||||||
| > | ||||||||||||||||||||||||
|
Comment on lines
+74
to
+78
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.
Suggested change
|
||||||||||||||||||||||||
| Show {isExpanded ? "less" : "more"} | ||||||||||||||||||||||||
| <CaretDown | ||||||||||||||||||||||||
| className={cn("size-3", isExpanded && "rotate-180")} | ||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||
| </button> | ||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||
| </Box> | ||||||||||||||||||||||||
| <Flex align="center" gap="1" className="shrink-0"> | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
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
isExpanded/isOverflowing/ResizeObserverpattern introduced here is near-identical to the one already present inChatThread.tsx'sUserBubble(lines 298–315), and a simpler variant exists inUserMessage.tsxtoo. This is now the third copy. Extracting auseOverflowClamp(contentRef, dep)hook would satisfy the OnceAndOnlyOnce rule, make the threshold (3lhvs5lh) a parameter, and give one place to fix the missingaria-expandedon the toggle button that all three share.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!