adding link rendering component#56
Conversation
WalkthroughAdds an EnhancedLinkWrapper that detects HTTP/HTTPS/IRC/IRCS links via regex and renders them as anchors by recursively processing React children. Replaces direct html content rendering with the wrapper for system, regular, and reply messages. MessageItem gains showHeader and setReplyTo props and imports updated. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as MessageItem
participant Wrapper as EnhancedLinkWrapper
participant React as React Children Utils
UI->>Wrapper: render(content)
Wrapper->>React: traverse children (Children.map / isValidElement)
React-->>Wrapper: return text nodes / elements
Wrapper->>Wrapper: split text by URL regex, create <a> for matches
Wrapper-->>UI: Fragment containing text and anchor nodes
sequenceDiagram
participant User
participant MessageItem
participant Handler as setReplyTo(msg)
User->>MessageItem: click "Reply"
MessageItem->>Handler: setReplyTo(message)
Handler-->>MessageItem: acknowledge (state updated)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/components/layout/ChatArea.tsx (5)
85-91: Make URL detection regex less error-prone (reduce false positives/negatives)The current regex can over/under-match (e.g., the unescaped dot after the host). Prefer a simpler, robust variant restricted to http/https, which covers most chat links reliably.
Apply this diff:
- const urlRegex = /(?:(?:https?|ftp):\/\/[^\s/$.?#].[^\s]*)/g; + // Basic, robust URL matcher for http/https links + const urlRegex = /\bhttps?:\/\/[^\s<>"']+/gi;If you need ftp and edge-cases (punycode, unicode TLDs), consider using linkify-it instead of maintaining a custom regex.
169-170: Avoid unnecessary wrapper div around linkified system contentRemoving the extra
prevents layout shifts in the flex row. The wrapper already returns inline-friendly content.Apply this diff:
- <div><EnhancedLinkWrapper>{htmlContent}</EnhancedLinkWrapper></div> + <EnhancedLinkWrapper>{htmlContent}</EnhancedLinkWrapper>
254-262: Linkify reply preview content for consistencyReply previews currently show raw text and won’t render links. Use the same mircToHtml + EnhancedLinkWrapper pipeline here.
Apply this diff:
┌ Replying to{" "} <strong>{message.replyMessage.userId.split("-")[0]}:</strong>{" "} - {message.replyMessage.content} + <EnhancedLinkWrapper> + {mircToHtml(message.replyMessage.content)} + </EnhancedLinkWrapper>
263-264: Avoid extra wrapper div around main message contentSame rationale as system messages: keep DOM shallow and content inline where possible.
Apply this diff:
- <div><EnhancedLinkWrapper>{htmlContent}</EnhancedLinkWrapper></div> + <EnhancedLinkWrapper>{htmlContent}</EnhancedLinkWrapper>
101-108: Use theme-consistent link colorsHard-coded blue may clash with themes. Using existing palette keeps visual consistency across light/dark or custom themes.
Apply this diff (already included in the main fix above; keeping it here for clarity):
- className="text-blue-500 underline hover:text-blue-700" + className="text-discord-text-link underline hover:opacity-80"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/components/layout/ChatArea.tsx(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/components/layout/ChatArea.tsx (2)
src/lib/ircUtils.tsx (2)
part(212-267)mircToHtml(167-279)src/types/index.ts (1)
Message(48-59)
🪛 GitHub Actions: Deploy to Cloudflare Pages
src/components/layout/ChatArea.tsx
[error] 118-118: TS7023: 'processChildren' implicitly has return type 'any' because it does not have a return type annotation and is referenced directly or indirectly in one of its return expressions.
[error] 119-119: TS7024: Function implicitly has return type 'any' because it does not have a return type annotation and is referenced directly or indirectly in one of its return expressions.
[error] 125-125: TS2769: No overload matches this call. The last overload gave the following error. Object literal may only specify known properties, and 'children' does not exist in type 'Partial & Attributes'.
🔇 Additional comments (1)
src/components/layout/ChatArea.tsx (1)
13-19: Icon additions: looks goodThe added icons (FaPenAlt, FaUserPlus) are imported and used appropriately in the header actions.
anotherdoesnm
changed the title
anotherdoesnm
changed the title
|
@ValwareIRC pls review this (i can't found how to make via gh interface) |
matheusfillipe
left a comment
matheusfillipe
left a comment
There was a problem hiding this comment.
This is approved and lgtm code wise also works ok.
However this is going to break tauri apps as they just follow hyperlinks leaving the app itself by default and we need to better handle that.
Please open an issue if this is merged as it is. We could then try to handle it with something like https://v2.tauri.app/plugin/opener/
|
Thanks for this. |
|
Added irc(s) to regex but for fully support we may need to add ability to specify query parameters and use https://developer.mozilla.org/en-US/docs/Web/API/Navigator/registerProtocolHandler |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/components/layout/ChatArea.tsx (4)
83-136: EnhancedLinkWrapper: solid recursive linkify (types + cloneElement usage are correct)
- Explicit return types address TS7023/TS7024.
- cloneElement usage is correct (children passed as the third arg).
- Skips existing anchors to avoid nesting.
- Returns a Fragment, preserving inline layout.
Nice work.
If you’d like, I can extract this into a reusable component file with lightweight unit tests.
104-104: Style consistency: avoid Tailwind’s blue-700; prefer theme-consistent hoverUse the theme token instead of hardcoding Tailwind blue. Suggest using hover:opacity-80 to keep the existing text-discord-text-link color while providing feedback.
Apply this diff:
- className="text-discord-text-link underline hover:text-blue-700" + className="text-discord-text-link underline hover:opacity-80"
86-114: Optional: handle trailing punctuation and closing brackets in detected URLsCurrent regex will include trailing punctuation like “),.” at the end of URLs, producing broken links. Consider iterating with RegExp.exec and trimming trailing punctuation from the match, while preserving it as text after the link.
Apply this diff to replace parseContent with a punctuation-aware version:
- const parseContent = (content: string): React.ReactNode[] => { - // Split the content based on the URL regex - const parts = content.split(urlRegex); - const matches = content.match(urlRegex) || []; - - return parts.map((part, index) => { - // Render the text part - const textPart = <span key={`text-${index}`}>{part}</span>; - - // If there's a matching link for this part, render it - if (index < matches.length) { - return ( - <Fragment key={`fragment-${index}`}> - {textPart} - <a - href={matches[index]} - target="_blank" - rel="noopener noreferrer" - className="text-discord-text-link underline hover:text-blue-700" - > - {matches[index]} - </a> - </Fragment> - ); - } - - return textPart; - }); - }; + const parseContent = (content: string): React.ReactNode[] => { + const nodes: React.ReactNode[] = []; + // Clone the regex to avoid shared lastIndex + const re = new RegExp(urlRegex.source, "gi"); + let lastIndex = 0; + let match: RegExpExecArray | null; + + while ((match = re.exec(content)) !== null) { + const start = match.index; + const raw = match[0]; + + // Push any text before the match + if (start > lastIndex) { + nodes.push( + <span key={`text-${lastIndex}`}>{content.slice(lastIndex, start)}</span>, + ); + } + + // Trim common trailing punctuation that shouldn't be part of the URL + const trimmed = raw.replace(/[)\]\.,;:!?]+$/, ""); + const trailing = raw.slice(trimmed.length); + + nodes.push( + <a + key={`link-${start}`} + href={trimmed} + target="_blank" + rel="noopener noreferrer" + className="text-discord-text-link underline hover:text-blue-700" + > + {trimmed} + </a>, + ); + + // Preserve any trimmed punctuation as plain text + if (trailing) { + nodes.push(<span key={`trail-${start}`}>{trailing}</span>); + } + + lastIndex = start + raw.length; + } + + // Push any remaining text after the last match + if (lastIndex < content.length) { + nodes.push( + <span key={`text-${lastIndex}`}>{content.slice(lastIndex)}</span>, + ); + } + + return nodes; + };
117-133: Flatten nested arrays from parseContent for predictable outputprocessChildren can return nested arrays when parseContent returns an array. Flattening keeps the structure predictable and avoids surprises in downstream renderers.
Apply this diff:
- const processChildren = (node: React.ReactNode): React.ReactNode[] => { - return Children.map(node, child => { - if (typeof child === 'string') { - return parseContent(child); // Process string content - } else if (isValidElement(child)) { - // Skip already-linkified anchors to avoid nested <a> - if ((child as React.ReactElement).type === 'a') { - return child; - } - // Directly process the children of the React element - const processed = processChildren((child as React.ReactElement).props?.children); - return cloneElement(child as React.ReactElement, undefined, processed); - } - // For other types of children, return them as is - return child as React.ReactNode; - }) ?? []; - }; + const processChildren = (node: React.ReactNode): React.ReactNode[] => { + const mapped = Children.map(node, (child) => { + if (typeof child === "string") { + return parseContent(child); + } else if (isValidElement(child)) { + // Skip already-linkified anchors to avoid nested <a> + if ((child as React.ReactElement).type === "a") { + return child as React.ReactNode; + } + const processed = processChildren( + (child as React.ReactElement).props?.children, + ); + return cloneElement(child as React.ReactElement, undefined, processed); + } + return child as React.ReactNode; + }) ?? []; + // Flatten nested arrays from parseContent + return mapped.flat(); + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/components/layout/ChatArea.tsx(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/components/layout/ChatArea.tsx (2)
src/lib/ircUtils.tsx (2)
mircToHtml(167-279)part(212-267)src/types/index.ts (1)
Message(48-59)
🔇 Additional comments (5)
src/components/layout/ChatArea.tsx (5)
4-4: Imports for React utilities are appropriateGood call adding Children, isValidElement, cloneElement, and Fragment; they’re all used correctly by EnhancedLinkWrapper.
13-19: New icons added are used and scoped properlyFaPenAlt and FaUserPlus are imported and consumed in the header. No dead imports.
170-170: System messages now linkify correctlyWrapping system htmlContent with EnhancedLinkWrapper ensures http/https/irc/ircs links are clickable. Good coverage.
261-261: Reply preview linkification looks goodApplying EnhancedLinkWrapper to the reply content addresses link-clickability in quoted previews.
264-264: Main message body linkification integratedWrapping the rendered htmlContent covers all normal messages. This matches the PR goal and includes ircs:// as requested.
Consider a quick manual check with messages containing:
- http(s)://, irc://, ircs://
- URLs followed by punctuation: “https://example.com),”
- Long URLs to ensure they wrap correctly in the UI
|
Great job |
adding link rendering component
3 participants
Working link rendering. Not sure if it's a right method
because i write without React knowledgebut... why not?Summary by CodeRabbit
New Features
Style
Refactor