Conversation
Add an Ask AI Tutor entrypoint in the RichText bubble menu. Connect it with the chat window flow for guided tutoring. Made-with: Cursor
adomingc
changed the title
✅ No New Circular DependenciesNo new circular dependencies detected. Current count: 0 |
🔍 Visual review for your branch is published 🔍Here are the links to: |
There was a problem hiding this comment.
Pull request overview
Adds an “Ask AI Tutor” entrypoint from the RichText bubble menu that opens a small floating AI chat seeded with the user’s selected text, and wires the NotesTextEditor + stories to support this new flow.
Changes:
- Extend
F0AiChatvisualization modes with a new"floating"mode and add a correspondingFloatingWindow. - Add RichText bubble menu “Ask AI Tutor” button + floating dialog (real CopilotKit-backed mode and a mock fallback).
- Expose tutor config/types via
NotesTextEditorand add a Storybook story demonstrating the tutor flow.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/sds/ai/F0AiChat/types.ts | Adds "floating" to the supported visualization modes. |
| packages/react/src/sds/ai/F0AiChat/components/layout/ChatWindow.tsx | Introduces FloatingWindow window wrapper with motion animations. |
| packages/react/src/sds/ai/F0AiChat/F0AiChat.tsx | Selects FloatingWindow vs SidebarWindow based on visualization mode. |
| packages/react/src/experimental/RichText/NotesTextEditor/index.tsx | Adds aiTutorConfig prop and exports tutor-related types/helpers. |
| packages/react/src/experimental/RichText/NotesTextEditor/index.stories.tsx | Adds WithAITutor story and sample readonly training content. |
| packages/react/src/experimental/RichText/CoreEditor/BubbleMenu/index.tsx | Adds Ask AI Tutor button integration and floating chat positioning. |
| packages/react/src/experimental/RichText/CoreEditor/BubbleMenu/AskAITutorDialog.tsx | New floating tutor chat (real + mock) and message injection helper. |
| packages/react/src/experimental/RichText/CoreEditor/BubbleMenu/AskAITutorButton.tsx | New bubble menu button used to open the tutor chat. |
| return messagesRef.current | ||
| .filter((m) => m.role === "user" || m.role === "assistant") | ||
| .map((m) => ({ | ||
| role: m.role as "user" | "assistant", | ||
| content: "content" in m ? String(m.content) : "", |
There was a problem hiding this comment.
serializeMessages() coerces CopilotKit message content via String(m.content), which will produce values like "[object Object]" for non-string content (CopilotKit can send structured/array content). Extract plain text content the same way the chat message renderer does, so the onGoDeeper callback receives usable transcripts.
| return messagesRef.current | |
| .filter((m) => m.role === "user" || m.role === "assistant") | |
| .map((m) => ({ | |
| role: m.role as "user" | "assistant", | |
| content: "content" in m ? String(m.content) : "", | |
| const extractPlainText = (content: unknown): string => { | |
| if (typeof content === "string") { | |
| return content | |
| } | |
| if (Array.isArray(content)) { | |
| return content | |
| .map((item) => extractPlainText(item)) | |
| .filter((part) => part) | |
| .join(" ") | |
| } | |
| if (typeof content === "object" && content !== null) { | |
| if ( | |
| "text" in content && | |
| typeof (content as { text: unknown }).text === "string" | |
| ) { | |
| return (content as { text: string }).text | |
| } | |
| } | |
| return "" | |
| } | |
| return messagesRef.current | |
| .filter((m) => m.role === "user" || m.role === "assistant") | |
| .map((m) => ({ | |
| role: m.role as "user" | "assistant", | |
| content: "content" in m ? extractPlainText(m.content) : "", |
| useEffect(() => { | ||
| if (!open && hasSentRef.current) { | ||
| onClose() | ||
| } | ||
| }, [open, onClose]) |
There was a problem hiding this comment.
The floating tutor container only calls onClose() when open becomes false and hasSentRef.current is true. If the user closes the chat before the initial message is sent (within the 500ms delay), the Copilot sidebar will close but the outer AskAITutorChat stays mounted (potentially leaving an empty 360x460 wrapper). Consider tracking "has been opened at least once" separately from "has sent" and close the outer container whenever the user closes the chat.
| setIsLoading(true) | ||
| setTimeout(() => { | ||
| setMessages((prev) => [ | ||
| ...prev, | ||
| { role: "assistant", content: getMockResponse(next) }, | ||
| ]) | ||
| setIsLoading(false) | ||
| }, 800) |
There was a problem hiding this comment.
In the mock tutor chat, handleSend schedules a setTimeout that is never cleared on unmount. Closing the tutor while a response is pending can trigger state updates after unmount. Store the timeout id in a ref and clear it in an effect cleanup or before scheduling a new timeout.
| (props.args.prompt as string) ?? | ||
| "Would you like me to explain this in more detail?" | ||
| } | ||
| confirmationText="Go deeper" | ||
| cancelText="No, thanks" |
There was a problem hiding this comment.
User-facing strings for the tutor "go deeper" confirmation (prompt/CTA/cancel) are hardcoded here rather than coming from i18n/config. In this repo, UI copy is generally sourced from useI18n() or passed via props so host apps can translate it (see packages/react/AGENTS.md i18n guidelines). Consider adding these strings to AiTutorLabels (or a dedicated labels object) and/or using useI18n() for defaults.
| aria-label={config.labels.buttonLabel} | ||
| title={config.labels.buttonTooltip} | ||
| className={cn( | ||
| "flex h-12 w-12 items-center justify-center rounded-full bg-white shadow-lg transition-transform hover:scale-110 active:scale-95", |
There was a problem hiding this comment.
This button hardcodes bg-white, which will likely look incorrect in dark mode and doesn't follow the design-token convention used elsewhere in this repo (bg-f1-*, border-f1-*, etc.). Use token-based background/shadow styles so the tutor button theme matches the rest of the editor UI.
| "flex h-12 w-12 items-center justify-center rounded-full bg-white shadow-lg transition-transform hover:scale-110 active:scale-95", | |
| "flex h-12 w-12 items-center justify-center rounded-full bg-f1-surface shadow-f1-floating transition-transform hover:scale-110 active:scale-95", |
| * Compact floating chat window — renders as a small card (360x460px). | ||
| * Used when visualizationMode is "floating". |
There was a problem hiding this comment.
The FloatingWindow JSDoc says it "renders as a small card (360x460px)", but the component itself doesn't enforce sizing (it uses h-full w-full and relies on the parent). Either enforce the dimensions here or update the comment to avoid implying a constraint that isn't implemented.
| * Compact floating chat window — renders as a small card (360x460px). | |
| * Used when visualizationMode is "floating". | |
| * Compact floating chat window used when visualizationMode is "floating". | |
| * The component fills the size provided by its parent container. |
| <MockAiTutorChat | ||
| selectedText={selectedText} | ||
| greeting={chatConfig?.greeting} | ||
| onGoDeeper={onGoDeeper} | ||
| /> |
There was a problem hiding this comment.
In mock mode, the floating tutor UI doesn't provide any way to close/dismiss the chat: MockAiTutorChat doesn't accept/use onClose, and the AskAITutorChat wrapper doesn't render a close control in this branch. If chatConfig.runtimeUrl is omitted (or in Storybook without a backend), users can get stuck with an always-open overlay. Consider adding a close button/escape handler for the mock branch and wiring it to onClose.
| setChatPosition({ | ||
| top: end.bottom - editorRect.top + 8, | ||
| left: Math.min( | ||
| start.left - editorRect.left, | ||
| editorRect.width - 370 // ensure chat doesn't overflow right | ||
| ), |
There was a problem hiding this comment.
The computed floating chat left position is only clamped with Math.min(...) and can become negative when the selection starts near the left edge (or when editorRect.width < 370), causing the tutor chat to render off-screen. Clamp left to a valid range (e.g. between 0 and editorRect.width - chatWidth) and avoid the hardcoded 370px offset by deriving it from the actual chat width.
| setChatPosition({ | |
| top: end.bottom - editorRect.top + 8, | |
| left: Math.min( | |
| start.left - editorRect.left, | |
| editorRect.width - 370 // ensure chat doesn't overflow right | |
| ), | |
| const chatWidth = 370 | |
| const rawLeft = start.left - editorRect.left | |
| const maxLeft = Math.max(0, editorRect.width - chatWidth) | |
| const clampedLeft = Math.max(0, Math.min(rawLeft, maxLeft)) | |
| setChatPosition({ | |
| top: end.bottom - editorRect.top + 8, | |
| left: clampedLeft, |
📦 Alpha Package Version PublishedUse Use |
| /** | ||
| * Compact floating chat window — renders as a small card (360x460px). | ||
| * Used when visualizationMode is "floating". | ||
| */ | ||
| export const FloatingWindow = ({ children }: WindowProps) => { | ||
| const { open } = useAiChat() | ||
|
|
||
| return ( | ||
| <AnimatePresence> | ||
| {open && ( | ||
| <motion.div | ||
| key="floating-chat" | ||
| className="pointer-events-auto flex h-full w-full flex-col overflow-hidden rounded-xl border border-solid border-f1-border-secondary bg-f1-special-page shadow-xl" | ||
| initial={{ opacity: 0, scale: 0.95, y: 10 }} | ||
| animate={{ opacity: 1, scale: 1, y: 0 }} | ||
| exit={{ opacity: 0, scale: 0.95, y: 10 }} | ||
| transition={{ duration: 0.2, ease: "easeOut" }} | ||
| > |
There was a problem hiding this comment.
FloatingWindow introduces motion animations but doesn’t respect the user’s “prefers-reduced-motion” setting. Per repo a11y conventions, please use useReducedMotion() (or equivalent) to disable/shorten transitions when reduced motion is requested.
| <div className="flex flex-col gap-2 rounded-xl border border-solid border-f1-border-secondary bg-f1-background p-3"> | ||
| <p className="text-sm text-f1-foreground-secondary">{text}</p> | ||
| <div className="flex gap-2"> | ||
| {onGoDeeper && ( | ||
| <button | ||
| type="button" | ||
| onClick={onGoDeeper} | ||
| className="flex items-center gap-1.5 rounded-lg border border-solid border-f1-border bg-f1-background px-3 py-2 text-sm font-medium text-f1-foreground transition-colors hover:bg-f1-background-secondary" | ||
| > | ||
| <svg | ||
| width="14" | ||
| height="14" | ||
| viewBox="0 0 16 16" | ||
| fill="none" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| > | ||
| <path | ||
| d="M6 3H3v10h10v-3M10 2h4v4M9 7l5-5" | ||
| stroke="currentColor" | ||
| strokeWidth="1.5" | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" | ||
| /> | ||
| </svg> | ||
| Go deeper | ||
| </button> | ||
| )} | ||
| {onQuizMe && ( | ||
| <button | ||
| type="button" | ||
| onClick={onQuizMe} | ||
| className="flex items-center gap-1.5 rounded-lg border border-solid border-f1-border bg-f1-background px-3 py-2 text-sm font-medium text-f1-foreground transition-colors hover:bg-f1-background-secondary" | ||
| > |
There was a problem hiding this comment.
New focusable controls inside AiTutorNextStepActions don’t include focusRing() (and rely only on hover styles). This makes keyboard focus hard to see and conflicts with repo accessibility conventions. Add focusRing() (or equivalent visible focus styles) to these buttons.
| useEffect(() => { | ||
| if (!injectedRef.current && messages.length > 0) { | ||
| setMessages( | ||
| messages.map((m) => ({ | ||
| id: randomId(), | ||
| role: m.role, | ||
| content: m.content, | ||
| })) | ||
| ) | ||
| setOpen(true) | ||
| injectedRef.current = true | ||
|
|
||
| // After injecting history, send a follow-up message to trigger a deeper explanation | ||
| setTimeout(() => { | ||
| sendMessage({ | ||
| id: randomId(), | ||
| role: "user", | ||
| content: | ||
| "Please go deeper into this topic. Provide a detailed explanation with examples, and include helpful resources like articles, videos, or documentation links.", | ||
| }) | ||
| }, 500) | ||
| } |
There was a problem hiding this comment.
AiTutorMessageInjector starts a setTimeout but doesn’t clear it on unmount. If the provider unmounts quickly, this can attempt to send a message after teardown. Store the timeout id and clear it in the effect cleanup (and consider guarding against double-send).
| chatConfig: { | ||
| runtimeUrl: "http://localhost:4111/copilotkit", | ||
| agent: "one-workflow", | ||
| greeting: | ||
| "Hello! I'm your AI tutor. Highlight any text in the training material and I'll help you understand it better.", | ||
| }, |
There was a problem hiding this comment.
This story hardcodes a localhost runtimeUrl, which will fail in CI/Chromatic/hosted Storybook and can make the story unusable. Prefer omitting runtimeUrl to exercise the built-in mock mode, or mock the network/runtime in Storybook (MSW) so the story is deterministic.
| const timer = setTimeout(() => { | ||
| const prompt = `<tool-context tool="ai-tutor">Please give a brief, concise explanation (3-4 sentences max) of the following text in simpler terms:</tool-context>${selectedText}` | ||
| sendMessage(prompt) | ||
| hasSentRef.current = true |
There was a problem hiding this comment.
The prompt is built by concatenating selectedText into a <tool-context> wrapper. Because selectedText can contain arbitrary HTML/text (including sequences like </tool-context>), this is vulnerable to prompt/tool-context injection. Escape or encode the selected text (e.g., wrap it in a JSON payload or otherwise sanitize/escape </>), and keep the tool-context section structurally unbreakable.
| // Mock mode: standalone chat UI | ||
| return ( | ||
| <div | ||
| className="absolute z-[9999] h-[460px] w-[360px]" | ||
| style={positionStyle} | ||
| > | ||
| <MockAiTutorChat | ||
| selectedText={selectedText} | ||
| greeting={chatConfig?.greeting} | ||
| onGoDeeper={onGoDeeper} | ||
| onQuizMe={onQuizMe} | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
In mock mode there is no way to close the tutor widget (no close button and no onClose wiring), so once opened it can only be removed by re-rendering the editor. Add a close affordance (button + Escape handler) and invoke the provided onClose callback.
| aria-label={config.labels.buttonLabel} | ||
| title={config.labels.buttonTooltip} | ||
| className={cn( | ||
| "flex h-12 w-12 items-center justify-center rounded-full bg-white shadow-lg transition-transform hover:scale-110 active:scale-95", |
There was a problem hiding this comment.
This new button uses a raw bg-white color. Styling guidelines prefer design tokens (bg-f1-*) for theming/dark mode consistency. Consider switching to the appropriate tokenized background color.
| "flex h-12 w-12 items-center justify-center rounded-full bg-white shadow-lg transition-transform hover:scale-110 active:scale-95", | |
| "flex h-12 w-12 items-center justify-center rounded-full bg-f1-surface-raised shadow-lg transition-transform hover:scale-110 active:scale-95", |
| type="button" | ||
| onClick={handleSend} | ||
| disabled={!input.trim() || isLoading} | ||
| className="flex h-10 w-10 flex-shrink-0 items-center justify-center rounded-xl bg-f1-background-accent-bold text-white transition-opacity disabled:opacity-40" |
There was a problem hiding this comment.
The mock chat’s send button is focusable but lacks visible focus styles (focusRing() or similar). Please add an explicit keyboard focus indicator to meet the repo’s focus visibility convention.
| className="flex h-10 w-10 flex-shrink-0 items-center justify-center rounded-xl bg-f1-background-accent-bold text-white transition-opacity disabled:opacity-40" | |
| className="flex h-10 w-10 flex-shrink-0 items-center justify-center rounded-xl bg-f1-background-accent-bold text-white transition-opacity focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-f1-border-bold focus-visible:ring-offset-2 focus-visible:ring-offset-f1-background disabled:opacity-40" |
Summary
Test plan
Made with Cursor