Basic Markdown support#183
Conversation
FirepitProject ID: Tip Sites auto-generate unique domains with the pattern https://randomstring.appwrite.network |
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe Changes
Sequence DiagramsequenceDiagram
participant Input as Message Text
participant Detector as Markdown Detector
participant Router as Render Router
participant Markdown as ReactMarkdown<br/>(with GFM)
participant Renderers as Custom Element<br/>Renderers
participant FallbackPath as RenderDecoratedText<br/>(Mentions & Emoji)
participant Output as Rendered Output
Input->>Detector: Check if markdown content
alt Contains Markdown
Detector->>Router: Is Markdown
Router->>Markdown: Process via ReactMarkdown
Markdown->>Renderers: Render custom elements
Renderers->>Renderers: Sanitize links (protocol whitelist)
Renderers->>Renderers: Replace images with alt placeholders
Renderers->>Renderers: Recursively apply mentions & emoji
Renderers->>Output: Return formatted output
else No Markdown
Detector->>Router: Not Markdown
Router->>FallbackPath: Use fallback path
FallbackPath->>FallbackPath: Apply mention tokens & emoji
FallbackPath->>Output: Return decorated text
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 unit tests (beta)
✨ Simplify code
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/search-results.tsx (1)
118-167:⚠️ Potential issue | 🟠 MajorAvoid rendering Markdown anchors inside the result button.
MessageWithMentionscan now emit<a>elements, so Markdown links in a search result create invalid nested interactive content inside this<button>. That can break keyboard/screen-reader behavior and make link clicks trigger row navigation instead. Disable link rendering for previews or move row navigation out of the ancestor of the message content. As per coding guidelines, Make sure all anchors are valid and navigable.🐛 Suggested direction
<div className="mt-1 text-muted-foreground text-sm"> <MessageWithMentions text={truncateText(message.text)} currentUserId="" customEmojis={customEmojis} + /* Prefer a prop such as renderLinks={false} here, + or render a non-interactive preview variant. */ /> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/search-results.tsx` around lines 118 - 167, The search result button currently contains MessageWithMentions which can emit <a> elements, causing nested interactive controls; fix by preventing link rendering inside the preview: update the MessageWithMentions invocation in search-results.tsx (the instance that uses truncateText(message.text)) to render plain text/no-anchors (e.g., pass a prop like renderLinks={false} or plainText={true} if supported) or, if MessageWithMentions has no such prop, replace it with a non-markdown/plain-text rendering of truncateText(message.text) so no <a> tags are produced; keep handleResultClick on the row button unchanged but ensure message preview contains no anchors so keyboard/screen-reader behavior and click routing remain correct.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/__tests__/components/message-with-mentions.test.tsx`:
- Around line 189-199: The test "should render basic markdown formatting" in
message-with-mentions.test.tsx currently mixes strong and strike-through which
can mask detection gaps for single-emphasis italics; update the test for the
MessageWithMentions component to include standalone italic cases using both
`_Italic_` and `*Italic*` (in separate render/assert blocks or additional
renders) and assert that each renders into an <em> element (e.g., via
screen.getByText("Italic").closest("em")) so the markdown detector for
single-emphasis is actually exercised.
In `@src/components/message-with-mentions.tsx`:
- Around line 18-21: The code currently tokenizes mentions into placeholders
(type MentionToken) before Markdown parsing but fails to restore them when
rendering code/pre/inline code nodes, causing placeholders like
FIREPITMENTIONTOKEN... to appear; update the renderer or AST walker used in
MessageWithMentions to detect code/pre/inlineCode nodes and replace any mention
placeholder substrings with their original MentionToken.token values (use the
existing MentionToken array or map you already create) before returning the
node's text/element; ensure this restoration runs in the same places you handle
normal text nodes (the code/pre branches referenced around lines ~18, ~194,
~263, ~382) so backticked mentions render correctly.
- Around line 13-14: The MARKDOWN_PATTERN currently misses single-emphasis
syntax (e.g., *italic* or _italic_), so messages that are valid Markdown but
only use single emphasis fall through to the plain-text branch; update the
MARKDOWN_PATTERN regex to include single-asterisk and single-underscore emphasis
alternatives (e.g., patterns matching \*...* and _..._) alongside the existing
bold/strikethrough/inline-link/list/blockquote/header cases so that
MARKDOWN_PATTERN correctly detects single-emphasis Markdown; modify the constant
MARKDOWN_PATTERN in src/components/message-with-mentions.tsx to add those
single-emphasis alternations.
---
Outside diff comments:
In `@src/components/search-results.tsx`:
- Around line 118-167: The search result button currently contains
MessageWithMentions which can emit <a> elements, causing nested interactive
controls; fix by preventing link rendering inside the preview: update the
MessageWithMentions invocation in search-results.tsx (the instance that uses
truncateText(message.text)) to render plain text/no-anchors (e.g., pass a prop
like renderLinks={false} or plainText={true} if supported) or, if
MessageWithMentions has no such prop, replace it with a non-markdown/plain-text
rendering of truncateText(message.text) so no <a> tags are produced; keep
handleResultClick on the row button unchanged but ensure message preview
contains no anchors so keyboard/screen-reader behavior and click routing remain
correct.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 46a3b3f8-7215-4b31-aa96-d6c0c42b925b
📒 Files selected for processing (3)
src/__tests__/components/message-with-mentions.test.tsxsrc/components/message-with-mentions.tsxsrc/components/search-results.tsx
|
@coderabbitai resolve |
✅ Actions performedComments resolved and changes approved. |
Merge activity
|
acarlson33
changed the base branch from
04-12-announcement_channel_type_implementation
to
graphite-base/183
acarlson33
force-pushed
the
04-12-basic_markdown_support
branch
from
8c1c998 to
2229c55
Compare
1 participant
This pull request adds robust Markdown support to the
MessageWithMentionscomponent, ensuring safe rendering of links and proper handling of mentions, emojis, and formatting. It also updates the test suite to cover these new behaviors and makes a minor UI adjustment in the search results.Markdown rendering and safety improvements:
src/components/message-with-mentions.tsx: Integratesreact-markdownwithremark-gfmto support basic Markdown formatting (bold, italic, strikethrough, lists, blockquotes, links, code, etc.) in messages. Ensures that only safe links (http, https, mailto, or relative) are rendered as clickable, with external links opening in a new tab and usingrel="noopener noreferrer"for security. Unsafe links (e.g.,javascript:) are not rendered as links. [1] [2]src/components/message-with-mentions.tsx: Mentions are tokenized and injected into the Markdown rendering flow, preserving highlighting and formatting even inside Markdown elements. Custom emoji rendering is disabled inside inline code blocks to prevent accidental parsing. [1] [2]Test coverage:
src/__tests__/components/message-with-mentions.test.tsx: Adds comprehensive tests for Markdown formatting, safe and unsafe link rendering, mention highlighting inside Markdown, and emoji handling inside code spans.UI consistency:
src/components/search-results.tsx: Changes the container for message previews from a<p>to a<div>to better support nested Markdown elements and avoid invalid HTML.