Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds link hover tooltips for rendered markdown and the rich-text editor; introduces editor link editing state with capturing click handling to open a link editor modal; refactors toolbar link flow to a modal-driven editor with URL normalization and keyboard shortcut support. Changes
Sequence DiagramssequenceDiagram
participant User
participant Markdown as Markdown Renderer
participant Tooltip as LinkHoverTooltip
participant Browser as Browser DOM
User->>Markdown: hover over rendered <a href>
Markdown->>Browser: provide anchor element and href
Tooltip->>Browser: measure anchor DOMRect
Tooltip->>Tooltip: compute tooltip position
Tooltip->>Browser: create portal and render URL tooltip
User->>Markdown: mouseout
Tooltip->>Browser: remove/hide tooltip
sequenceDiagram
participant User
participant Toolbar as RichText Toolbar
participant Context as RichTextContext
participant Editor as TipTap Editor
participant Modal as Link Editor Modal
User->>Toolbar: click link button or press Ctrl/Cmd+K
Toolbar->>Context: setLinkEditorOpen(true)
Context->>Modal: effect computes range/text/href and opens modal
User->>Modal: Save or Delete
Modal->>Editor: normalize URL and apply/unset link mark
Modal->>Context: setLinkEditorOpen(false)
Context->>Editor: resume editing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/ui/src/rich-text-area/rich-text-toolbar.tsx (1)
163-182: Consider: Canceling the prompt removes an existing link.When the user clicks Cancel on the prompt (returning
null), the current logic callsunsetLink(), which removes an existing link. This may be unexpected if the user simply wanted to dismiss the dialog without making changes.If preserving the link on cancel is desired, you could check for
url === null(cancel) vsurl === ""(cleared input):💡 Optional fix to preserve link on cancel
useEffect(() => { if (!editor || !linkEditorOpen) return; const previousUrl = editor.getAttributes("link").href; const url = window.prompt("Link URL", previousUrl); + + // User cancelled the prompt + if (url === null) { + setLinkEditorOpen(false); + return; + } + const normalizedUrl = url ? normalizeLinkUrl(url) : url; - if (!normalizedUrl?.trim()) { + if (!normalizedUrl.trim()) { editor.chain().focus().extendMarkRange("link").unsetLink().run(); } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/rich-text-area/rich-text-toolbar.tsx` around lines 163 - 182, The prompt cancel logic currently treats a null result the same as an empty string and calls editor.chain().focus().extendMarkRange("link").unsetLink(), removing existing links; modify the effect handling for linkEditorOpen so it distinguishes url === null (user canceled) from url === "" (user cleared input). Specifically, in the useEffect that reads previousUrl via editor.getAttributes("link").href and calls normalizeLinkUrl, check for url === null and simply close the editor (setLinkEditorOpen(false)) without calling unsetLink or setLink; only call unsetLink when url === "" or normalizedUrl is an empty/whitespace string, and call setLink({ href: normalizedUrl }) when a valid normalizedUrl exists. Ensure you reference editor, normalizeLinkUrl, unsetLink, setLink, and linkEditorOpen in the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ui/src/rich-text-area/index.tsx`:
- Around line 160-184: The effect creates an infinite loop because it depends on
tooltipState and calls setTooltipState with a fresh rect object from updateRect;
fix by changing the dependency array to [href, !!tooltipState] (boolean presence
of tooltipState) instead of tooltipState, and remove the immediate call to
updateRect() inside the effect (rely on the existing onMouseEnter/onMouseLeave
logic to set tooltipState initially); keep updateRect, containerRef,
setTooltipState and the resize/scroll listeners and their cleanup as-is.
---
Nitpick comments:
In `@packages/ui/src/rich-text-area/rich-text-toolbar.tsx`:
- Around line 163-182: The prompt cancel logic currently treats a null result
the same as an empty string and calls
editor.chain().focus().extendMarkRange("link").unsetLink(), removing existing
links; modify the effect handling for linkEditorOpen so it distinguishes url ===
null (user canceled) from url === "" (user cleared input). Specifically, in the
useEffect that reads previousUrl via editor.getAttributes("link").href and calls
normalizeLinkUrl, check for url === null and simply close the editor
(setLinkEditorOpen(false)) without calling unsetLink or setLink; only call
unsetLink when url === "" or normalizedUrl is an empty/whitespace string, and
call setLink({ href: normalizedUrl }) when a valid normalizedUrl exists. Ensure
you reference editor, normalizeLinkUrl, unsetLink, setLink, and linkEditorOpen
in the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 00c99faf-175c-427a-b342-a8f0425c44ff
📒 Files selected for processing (4)
apps/web/ui/messages/message-markdown.tsxpackages/ui/src/rich-text-area/index.tsxpackages/ui/src/rich-text-area/rich-text-provider.tsxpackages/ui/src/rich-text-area/rich-text-toolbar.tsx
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ui/src/rich-text-area/rich-text-toolbar.tsx`:
- Around line 358-391: The label elements for the "Text" and "Link" fields are
not associated with their Input components (so screen readers won't announce
them); fix by adding matching id/htmlFor pairs or by wrapping each <Input>
inside its <label> for the Text and Link blocks — for example, give the Text
Input an id (e.g., text-input-unique) and set the label's htmlFor to that id,
and likewise give the Link Input an id (ensure it doesn't break linkInputRef
usage) and set its label's htmlFor to that id; keep existing handlers (onChange
using textValue/urlValue, onKeyDown calling saveLink) unchanged.
- Around line 69-78: The loop that expands the selection using linkMark (the
while blocks that adjust from/to and call state.doc.rangeHasMark) must also
verify the neighboring link mark's attributes match the current link's attrs
(e.g., href) before extending; update both loops to fetch the mark instance at
the adjacent position (via the doc resolve/node marks APIs) and compare its
attrs.href (or full attrs) to the original link mark's attrs and stop expanding
if they differ so back-to-back links with different hrefs are not merged into
one editable range.
- Around line 319-334: The current sequence always calls insertContentAt with
nextText which replaces the selected slice and strips inline marks; change the
logic in the toolbar handler so that before calling editor.chain().focus() you
compare the existing selected text (from selectionState.from..to) to nextText
and only call insertContentAt when they differ; if they are identical, skip
insertContentAt and only set the text selection (using setTextSelection with
from: selectionState.from, to: selectionState.from + nextText.length) and then
call setLink({ href: normalizedUrl }) to apply the link without rewriting the
node content, ensuring inline marks are preserved (update the code paths around
insertContentAt, setTextSelection, and setLink to branch on the equality check).
- Around line 27-34: normalizeLinkUrl currently accepts any URI scheme via
/^[a-z][a-z0-9+.-]*:/i which allows dangerous schemes like javascript: and
data:; update normalizeLinkUrl to only allow a safe scheme allowlist (at minimum
"http", "https", "mailto"), keep existing trimming and protocol-relative
handling (the startsWith("//") -> "https:" case) and when a scheme is present
but not in the allowlist return an empty string (or otherwise treat as invalid)
instead of returning the original URL, and when no scheme is present continue to
prepend "https://" to form a safe URL; ensure this logic is implemented inside
normalizeLinkUrl so callers (e.g., toolbar link insertion) receive only
allowlisted schemes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 17ca9e38-2a6a-4393-b14d-f66a2bdb5df1
📒 Files selected for processing (2)
packages/ui/src/rich-text-area/rich-text-provider.tsxpackages/ui/src/rich-text-area/rich-text-toolbar.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ui/src/rich-text-area/rich-text-provider.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/ui/src/rich-text-area/rich-text-toolbar.tsx (2)
433-441: Consider using theButtoncomponent for consistency.The "Delete link" action uses a plain
<button>with inline styles while "Cancel" and "Save" use theButtoncomponent. UsingButtonwithvariant="secondary"or a destructive variant would ensure consistent styling and behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/rich-text-area/rich-text-toolbar.tsx` around lines 433 - 441, Replace the plain <button> used for the "Delete link" action with the shared Button component to keep styling and behavior consistent; locate the block gated by selectionState.isLink and swap the element that calls deleteLink to use Button (e.g., Button with variant="secondary" or a destructive variant/prop your Button supports), preserving the onClick handler, type="button", and aria/labeling semantics so the functionality and accessibility remain unchanged.
242-243: UseuseId()to avoid ID collisions with multiple editor instances.These static IDs will conflict if multiple
RichTextToolbarcomponents are rendered on the same page, potentially causing accessibility issues where clicking a label focuses the wrong input.Proposed fix using React's useId hook
+ import { useId } from "react"; // ... in LinkButton component: - const textInputId = "rich-text-link-text-input"; - const linkInputId = "rich-text-link-url-input"; + const id = useId(); + const textInputId = `${id}-text`; + const linkInputId = `${id}-url`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/rich-text-area/rich-text-toolbar.tsx` around lines 242 - 243, The static IDs textInputId and linkInputId in the RichTextToolbar component cause collisions for multiple instances; change the component to call React's useId() (import useId from React) to generate a unique baseId inside the RichTextToolbar function, then derive textInputId and linkInputId from that base (e.g., `${baseId}-link-text` and `${baseId}-link-url`) and replace the hardcoded strings wherever textInputId and linkInputId are used (labels, inputs, aria attributes) to ensure each toolbar instance has unique, accessible IDs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/ui/src/rich-text-area/rich-text-toolbar.tsx`:
- Around line 433-441: Replace the plain <button> used for the "Delete link"
action with the shared Button component to keep styling and behavior consistent;
locate the block gated by selectionState.isLink and swap the element that calls
deleteLink to use Button (e.g., Button with variant="secondary" or a destructive
variant/prop your Button supports), preserving the onClick handler,
type="button", and aria/labeling semantics so the functionality and
accessibility remain unchanged.
- Around line 242-243: The static IDs textInputId and linkInputId in the
RichTextToolbar component cause collisions for multiple instances; change the
component to call React's useId() (import useId from React) to generate a unique
baseId inside the RichTextToolbar function, then derive textInputId and
linkInputId from that base (e.g., `${baseId}-link-text` and
`${baseId}-link-url`) and replace the hardcoded strings wherever textInputId and
linkInputId are used (labels, inputs, aria attributes) to ensure each toolbar
instance has unique, accessible IDs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 47b8e856-5565-4c13-b373-42770b2344c3
📒 Files selected for processing (1)
packages/ui/src/rich-text-area/rich-text-toolbar.tsx
Composing messages
Reading messages
CleanShot.2026-04-13.at.15.24.46.mp4
Summary by CodeRabbit
New Features
Bug Fixes