WIP: clickable results #395
Conversation
Reviewer's Guide by SourceryThis pull request introduces clickable text functionality to the chat messages, allowing users to generate follow-up queries by clicking on specific parts of the text. It includes the implementation of Sequence diagram for generating follow-up queries from clickable textsequenceDiagram
participant User
participant ChatMessage
participant ClickableText
participant sendMessageFromResponse
User->>ChatMessage: Clicks on clickable text
ChatMessage->>ClickableText: Handles click event with text
ClickableText->>sendMessageFromResponse: Calls sendMessageFromResponse(text)
sendMessageFromResponse-->>ChatMessage: Sends new query
ChatMessage-->>User: Displays response
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces extensive enhancements to the styling and interactivity of clickable text elements within the application. New CSS classes and hover/focus effects improve button and heading aesthetics. Several React components have been added or refactored—most notably, the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant ECT as EnhancedClickableText
participant UF as Utility Functions
participant CB as Callback (sendMessageFromResponse)
U->>ECT: Clicks on a text element
ECT->>UF: Processes text & extracts clickable portion
UF-->>ECT: Returns processed text data
ECT->>CB: Invokes callback with formatted query
CB-->>ECT: Acknowledges the response
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
apps/masterbots.ai/app/globals.cssOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/apps/masterbots.ai/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. apps/masterbots.ai/components/routes/chat/chat-clickable-list.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/apps/masterbots.ai/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. apps/masterbots.ai/components/routes/chat/chat-clickable-text.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/apps/masterbots.ai/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team.
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Bran18 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a fallback mechanism to
processContentto handle unexpected React element types gracefully. - The
UNIQUE_PHRASESconstant is duplicated inlib/utils.tsandlib/clickable-results.ts; consolidate it into a single source of truth.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 2 issues found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const contentText = | ||
| typeof children === 'string' | ||
| ? children | ||
| : extractTextFromReactNodeNormal(children) | ||
|
|
There was a problem hiding this comment.
issue (complexity): Consider using a lookup table to map element types to processors to reduce nested conditionals in processContent.
It looks like you still have many helper functions and deeply nested conditionals scattered throughout the code. To reduce complexity, consider consolidating similar processing rules into a lookup table that maps element types to their corresponding processors. This can clean up the main processContent function and lower the depth of nested conditionals.
For example, you could extract this mapping:
const elementProcessors: Record<string, (elem: React.ReactElement) => React.ReactNode> = {
// Headings
'h1': processHeading,
'h2': processHeading,
'h3': processHeading,
'h4': processHeading,
'h5': processHeading,
'h6': processHeading,
// Emphasis
'strong': processEmphasis,
'b': processEmphasis,
// List item
'li': processListItem,
// Links
'a': processLink,
};And then refactor your processContent function like so:
if (React.isValidElement(content) && typeof content.type === 'string') {
const processor = elementProcessors[content.type];
if (processor) {
return processor(content);
}
// If element has children, process them recursively.
if (content.props?.children) {
return React.cloneElement(content, {
...content.props,
children: processContent(content.props.children),
});
}
}This change reduces the number of direct if/else checks, making the logic easier to follow while keeping functionality intact.
| } | ||
|
|
||
| // Process React elements | ||
| if (React.isValidElement(content)) { |
There was a problem hiding this comment.
issue (complexity): Consider using a configurable transformation list to handle different element types, reducing nested conditionals in processContent.
Consider abstracting the conditional branching into a configurable transformation list so that you avoid writing separate deep branches. For example, you could create an array of transformation objects that encapsulates the conditions and transformation functions. Then in your recursive processContent, check if any transformation applies before processing children. This keeps the recursion logic flatter and more declarative.
For instance, you can refactor like this:
// List your processors
const processors: {
check: (element: React.ReactElement) => boolean;
transform: (element: React.ReactElement) => React.ReactElement;
}[] = [
{
check: (element) => isHeading(element),
transform: (element) => processSectionHeading(element),
},
{
check: (element) => isListItem(element),
transform: (element) => processListItemWithBold(element),
},
// add more processors as needed
];
const processContent = (content: React.ReactNode): React.ReactNode => {
if (typeof content === 'string' || typeof content !== 'object' || content === null)
return content;
if (Array.isArray(content))
return content.map((item, index) => (
<React.Fragment key={`content-item-${index}`}>
{processContent(item)}
</React.Fragment>
));
if (React.isValidElement(content)) {
// Find the first matching processor
for (const { check, transform } of processors) {
if (check(content)) {
return transform(content);
}
}
// Process children if available
if (content.props?.children) {
return React.cloneElement(content, {
...content.props,
children: processContent(content.props.children),
});
}
}
return content;
};This approach maintains functionality while reducing nested conditional logic.
| // Process standard text nodes (paragraphs, plain text) | ||
| const processTextContent = (text: string) => { | ||
| // Don't process empty or very short text | ||
| if (!text || text.length < 3) return text |
There was a problem hiding this comment.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces)
| if (!text || text.length < 3) return text | |
| if (!text || text.length < 3) { |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
| * Extracts text content from React nodes | ||
| */ | ||
| export function extractTextContent(node: ReactNode): string { | ||
| if (typeof node === 'string') return node |
There was a problem hiding this comment.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces)
| if (typeof node === 'string') return node | |
| if (typeof node === 'string') { |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
| */ | ||
| export function extractTextContent(node: ReactNode): string { | ||
| if (typeof node === 'string') return node | ||
| if (typeof node === 'number') return node.toString() |
There was a problem hiding this comment.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces)
| if (typeof node === 'number') return node.toString() | |
| if (typeof node === 'number') { |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
| export function extractClickablePortion(text: string): string { | ||
| // For markdown headings | ||
| const headingMatch = text.match(/^(#+\s+[^:\n]+)/) | ||
| if (headingMatch) return headingMatch[1].trim() |
There was a problem hiding this comment.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces)
| if (headingMatch) return headingMatch[1].trim() | |
| if (headingMatch) { |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
|
|
||
| // For numbered sections | ||
| const numberedMatch = text.match(/^(\d+\.\s+[^:\n]+)/) | ||
| if (numberedMatch) return numberedMatch[1].trim() |
There was a problem hiding this comment.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces)
| if (numberedMatch) return numberedMatch[1].trim() | |
| if (numberedMatch) { |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
|
|
||
| // For sections with colons (Title: Description) | ||
| const colonMatch = text.match(/^([^:]+):/) | ||
| if (colonMatch) return colonMatch[1].trim() |
There was a problem hiding this comment.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces)
| if (colonMatch) return colonMatch[1].trim() | |
| if (colonMatch) { |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
|
|
||
| // For bullet points | ||
| const bulletMatch = text.match(/^([•-]\s+[^:\n]+)/) | ||
| if (bulletMatch) return bulletMatch[1].trim() |
There was a problem hiding this comment.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces)
| if (bulletMatch) return bulletMatch[1].trim() | |
| if (bulletMatch) { |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
|
|
||
| // For bold text | ||
| const boldMatch = text.match(/^\*\*([^*]+)\*\*/) | ||
| if (boldMatch) return boldMatch[1].trim() |
There was a problem hiding this comment.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces)
| if (boldMatch) return boldMatch[1].trim() | |
| if (boldMatch) { |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (16)
apps/masterbots.ai/lib/utils.ts (1)
340-355: Consider externalizing or documenting phrase usage.The new
UNIQUE_PHRASESarray is straightforward. However, if these phrases influence broader application logic or user-facing text, consider externalizing them into a config or an i18n file. This keeps the codebase tidier and the usage more discoverable.apps/masterbots.ai/components/routes/chat/chat-message.tsx (1)
91-131: Ensure performance with large Markdown content.Replacing direct rendering with
<EnhancedContent>across paragraphs, headings, and strong tags effectively injects clickable features. However, watch for potential performance hits if messages become very large, as nested React elements can grow in complexity.apps/masterbots.ai/lib/clickable-content-extraction.ts (2)
22-32: Regex heading extraction looks good.
extractTitleFromHeadinguses a concise regex to strip numeric prefixes. For headings lacking a trailing period, the function falls back to the original text. This is a suitable fallback; just be mindful of variant heading formats.
34-52: Evaluate case sensitivity for context extraction.
extractContextFromTextpinpoints text from the first occurrence of the keyword until the colon. If you need a case-insensitive match or a different delimiter, consider expanding the logic.apps/masterbots.ai/types/types.ts (1)
91-92: Consider replacinganywith a more specific type.Using
anyhere could mask potential type issues. Defining a more precise type (e.g.,numberor a union of acceptable types) will improve type safety and clarity.// biome-ignore lint/suspicious/noExplicitAny: <explanation> - meter: any + // TODO: Provide a more specific type, for example: + meter: number | nullapps/masterbots.ai/components/routes/chat/chat-clickable-list.tsx (2)
53-53: Avoid usinganyforcontextText.Currently,
contextTextis typed asany, which could lead to losing valuable typing information. Consider switching to a well-defined type or a more generic structure if the shape is unknown.- const handleClick = (text: string, contextText: any = '') => { + interface ContextData { + // Provide fields for the context if known + details?: string; + } + + const handleClick = (text: string, contextText: ContextData = {}) => {
169-224: Consider short-circuit returns inprocessContentfor large nested structures.This recursive logic is fine for moderate nesting, but deep or invalid structures could cause performance issues. Early checks or short-circuit conditions can improve resiliency.
apps/masterbots.ai/components/routes/chat/chat-clickable-text.tsx (3)
38-38: Parameterize the follow-up prompt.Currently, the prompt text is hardcoded to “Explain more in-depth and in detail about…”. For flexibility and maintainability, consider externalizing or parameterizing this phrase.
- sendMessageFromResponse(`Explain more in-depth and in detail about ${cleanedText}`) + const followUpPrompt = `Explain more in-depth and in detail about ${cleanedText}` + sendMessageFromResponse(followUpPrompt)
48-83: Centralize content-type checks.
getContentTypeClassuses multiple regex matches and conditionals. If the checks expand, consider moving them into a separate helper function or object map for clarity and maintainability.
304-306: Clarify thenodeprop usage.When
nodeis present, the code delegates toprocessContent(children). Ifnodecan contain its own logic or differ fromchildren, consider clarifying or renaming to avoid confusion.apps/masterbots.ai/lib/clickable-results.ts (5)
6-41: Consider React.cloneElement or alternative approach for React element transformation.Reconstructing the React element with a spread operator might lose some metadata (e.g.,
key, ref handling). A safer approach is to useReact.cloneElementor a dedicated utility that preserves all element properties unless a full recreation is intentionally desired.
43-67: DRY opportunity withextractTextFromReactNodeWeb.This function closely mirrors the logic in
extractTextFromReactNodeWeb; consider consolidating both into a shared utility to reduce code duplication and possible maintenance burdens.
69-81: Include other punctuation incleanClickableText.Only a narrow set of trailing characters is stripped. If you want to remove “?”, “!”, etc., expand the character class.
You could do:
- .replace(/[,.()[\]]$/, '') // Remove trailing punctuation + .replace(/[,.()[\]?!]$/, '') // Also remove question marks, exclamation marks
105-130: Unify similar pattern checks inextractClickablePortion.Multiple sequential regex checks make the function straightforward but somewhat repetitive. Storing them as a list of patterns and iterating might simplify future extensions and improve maintainability.
132-256: Refactor largeparseClickableTextfunction.The multi-pattern matching logic can be extracted into smaller helper functions or a config-driven approach. This will reduce complexity and streamline updates.
apps/masterbots.ai/app/globals.css (1)
266-280: Ensure consistent styling across.text-linkand.clickable-text.Defining button styles for both
.text-linkand.clickable-textclasses in tandem is good for reusability, but check if you need to unify them or use a single class to avoid CSS bloat or conflicting rules over time.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/masterbots.ai/app/globals.css(3 hunks)apps/masterbots.ai/components/routes/chat/chat-clickable-list.tsx(1 hunks)apps/masterbots.ai/components/routes/chat/chat-clickable-text.tsx(2 hunks)apps/masterbots.ai/components/routes/chat/chat-message.tsx(4 hunks)apps/masterbots.ai/lib/clickable-content-extraction.ts(1 hunks)apps/masterbots.ai/lib/clickable-results.ts(1 hunks)apps/masterbots.ai/lib/utils.ts(1 hunks)apps/masterbots.ai/types/types.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
apps/masterbots.ai/lib/clickable-results.ts (1)
Learnt from: AndlerRL
PR: bitcashorg/masterbots#388
File: apps/masterbots.ai/lib/clickable-results.ts:148-149
Timestamp: 2025-02-24T20:46:17.229Z
Learning: Issue #389 is tracking potential improvements and refactoring needs for the ClickableText component, including the evaluation of using context objects instead of multiple parameters.
🔇 Additional comments (19)
apps/masterbots.ai/components/routes/chat/chat-message.tsx (7)
10-17: Doc comment aligns well with the new naming.Renaming to
EnhancedChatMessageand adding the descriptive JSDoc is coherent with the new functionality. Clear documentation helps maintainers understand the purpose of this enhanced version.
19-40: Validate the references for assistant vs user roles.In
EnhancedChatMessage, you're now cleaning the content and managing references. Verify that references are only relevant to non-user roles and that undefined references don't break the rendering.
71-83: EnhancedContent wrapper preserves user text behavior.Wrapping children with
EnhancedClickableTextis a neat approach to selectively enable click handling for assistant messages only. This ensures the user’s own text isn't inadvertently made clickable.
133-149: List handling is clear.Applying the
<EnhancedContent>logic to list elements is consistent. The condition to detect nested lists is a good safeguard. This code segment looks solid.
153-166: Link rendering is correct and secure.Providing
target="_blank"andrel="noopener noreferrer"is a good practice to prevent security vulnerabilities. This safeguards your users when opening external links.
168-197: Code block handling is well-structured.Integrating
CodeBlockfor multi-line code snippets is a robust approach. The check for inline vs block code, along with language extraction, appears correctly implemented.
202-206: Conditional rendering of ChatMessageActions.Conditionally displaying actions with
actionRequiredis a clean design choice. It ensures clarity of when user actions are required, keeping the UI minimal otherwise.apps/masterbots.ai/lib/clickable-content-extraction.ts (4)
1-3: Imports and setup confirmed.The import of React,
ReactNodetypes, and usage match the file’s purpose. No issues are detected here.
4-20: Handle React fragments if needed.
extractTextContentworks well for common node types. If fragment usage is expected (<></>), consider a fallback to handle them explicitly, though your current checks likely suffice.
58-63: Consider validating or escaping the query inputs.
formatFollowUpQueryis straightforward, but if the user can supply arbitrarykeywordorcontext, consider sanitizing or validating them to avoid potential injection or formatting issues.
65-75: Safe invoke pattern is neat.
safeInvokeneatly prevents runtime errors by checking if the callback is a function. This is a clean defensive coding practice.apps/masterbots.ai/types/types.ts (1)
361-366: Nice addition ofParsedTextinterface!The fields are well-labeled, making it clear how processed text is structured. No issues found here.
apps/masterbots.ai/components/routes/chat/chat-clickable-list.tsx (2)
63-96:processSectionHeadinglogic looks good!Gracefully handles the click event on headings and applies hover styling & accessibility attributes. No concerns here.
98-167: ValidatestrongTextextraction inprocessListItemWithBold.Ensure that
strongTextdoesn't remain empty if the node is unexpectedly empty or malformed. You might guard the re-cloning logic with a check for valid text to prevent potential runtime issues.apps/masterbots.ai/components/routes/chat/chat-clickable-text.tsx (1)
141-178: Smooth handling of emphasized text with colons.The logic for splitting on the first colon is clear and covers common AI-generated subsection headings. No issues found.
apps/masterbots.ai/lib/clickable-results.ts (2)
4-4:UNIQUE_PHRASESimport is preserved—verify necessity.The import statement remains present despite references suggesting prior removal. Confirm that
UNIQUE_PHRASESis indeed required to avoid unused imports or inconsistencies across the codebase.
82-103: Clarify URL matching logic.This function discards URLs but may inadvertently skip other link-like patterns (e.g., relative or protocol-less URLs). Verify whether that is acceptable for your usage scenario.
apps/masterbots.ai/app/globals.css (2)
281-326: Hover and focus interactions look good.The addition of transitions, hover underlines, and focus outlines provides a clear interactive experience. Keep an eye on potential layering issues if other components introduce additional focus or hover states.
603-653: Verify broader browser compatibility and test style overrides.The specialized heading, bullet, and unique phrase styling significantly improves readability and clarity. Ensure that older or less common browsers correctly handle the
::afterunderline transition and data attributes (data-section-title).
Summary by Sourcery
Implement clickable text functionality in chat messages to allow users to generate follow-up queries by clicking on key sections of the text. This includes identifying and handling clickable portions of text, such as headings, list items, and bolded text.
New Features:
Summary by CodeRabbit