-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: open in chat button in doc examples #4996
base: canary
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request integrates a new chat feature into the docs app. New environment variables are added to configure the chat API. An asynchronous function ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CodeDemo
participant OpenInChat as openInChat Function
participant ChatAPI as Chat API
User->>CodeDemo: Click "Open in Chat"
CodeDemo->>OpenInChat: Call openInChat({title, files})
OpenInChat->>files: Retrieve `/App.jsx` content
OpenInChat->>OpenInChat: Validate content and prepend React import if missing
OpenInChat->>OpenInChat: Parse dependencies via parseDependencies
OpenInChat->>ChatAPI: POST request (title, content, dependencies)
ChatAPI-->>OpenInChat: Return chat URL or error
OpenInChat-->>CodeDemo: Return result (chat URL or error message)
CodeDemo->>User: Open new window with chat URL (or display error)
Suggested labels
Suggested reviewers
✨ Finishing Touches
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:
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
Documentation and Community
|
@heroui/accordion
@heroui/alert
@heroui/autocomplete
@heroui/avatar
@heroui/badge
@heroui/breadcrumbs
@heroui/button
@heroui/calendar
@heroui/card
@heroui/checkbox
@heroui/chip
@heroui/code
@heroui/date-input
@heroui/date-picker
@heroui/divider
@heroui/drawer
@heroui/dropdown
@heroui/form
@heroui/image
@heroui/input
@heroui/input-otp
@heroui/kbd
@heroui/link
@heroui/listbox
@heroui/menu
@heroui/modal
@heroui/navbar
@heroui/number-input
@heroui/pagination
@heroui/popover
@heroui/progress
@heroui/radio
@heroui/ripple
@heroui/scroll-shadow
@heroui/select
@heroui/skeleton
@heroui/slider
@heroui/snippet
@heroui/spacer
@heroui/spinner
@heroui/switch
@heroui/table
@heroui/tabs
@heroui/toast
@heroui/tooltip
@heroui/user
@heroui/react
@heroui/system
@heroui/system-rsc
@heroui/theme
@heroui/use-aria-accordion
@heroui/use-aria-accordion-item
@heroui/use-aria-button
@heroui/use-aria-link
@heroui/use-aria-modal-overlay
@heroui/use-aria-multiselect
@heroui/use-callback-ref
@heroui/use-clipboard
@heroui/use-data-scroll-overflow
@heroui/use-disclosure
@heroui/use-draggable
@heroui/use-image
@heroui/use-infinite-scroll
@heroui/use-intersection-observer
@heroui/use-is-mobile
@heroui/use-is-mounted
@heroui/use-measure
@heroui/use-pagination
@heroui/use-real-shape
@heroui/use-ref-state
@heroui/use-resize
@heroui/use-safe-layout-effect
@heroui/use-scroll-position
@heroui/use-ssr
@heroui/use-theme
@heroui/use-update-effect
@heroui/aria-utils
@heroui/dom-animation
@heroui/framer-utils
@heroui/react-rsc-utils
@heroui/react-utils
@heroui/shared-icons
@heroui/shared-utils
@heroui/stories-utils
@heroui/test-utils
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (5)
apps/docs/components/docs/components/code-demo/parse-dependencies.ts (2)
1-2
: Consider adding more robust package detectionThe regex pattern looks good for basic import/from statements but might miss complex imports or dynamic imports.
Consider enhancing the regex to handle more edge cases or adding a comment about its limitations. For complex projects, a more robust parsing approach might be needed in the future.
30-33
: Consider making fixedVersions more maintainableThe hardcoded version mapping works but could become difficult to maintain as dependencies grow.
Consider moving this to a separate configuration file that can be more easily maintained, possibly with comments explaining why specific versions are required.
apps/docs/actions/open-in-chat.ts (1)
9-19
: Add type declaration for return valueThe function's return type should be explicitly defined for better type safety and documentation.
- export const openInChat = async ({title, files}: {title?: string; files: SandpackFiles}) => { + export const openInChat = async ({title, files}: {title?: string; files: SandpackFiles}): Promise<{error: string | null; data: string | null}> => {apps/docs/components/docs/components/code-demo/code-demo.tsx (2)
178-178
: Consider a more robust path check.Using
pathname.includes("/components/")
works but could potentially catch false positives if there are other paths containing this string.- const isComponentsPage = pathname.includes("/components/"); + const isComponentsPage = /^\/components\/[^\/]+/.test(pathname);
180-212
: Well-implemented chat integration with analytics tracking.The implementation handles the complete flow well: capturing analytics events, managing loading state, handling errors, and opening the chat in a new window.
A few suggestions for improvement:
- The component name extraction logic could be more robust for handling kebab-case paths:
- const component = pathname.split("/components/")[1]; + // Handle paths with trailing slash and extract just the component segment + const componentSegment = pathname.split("/components/")[1]?.split("/")[0] || ""; + const component = componentSegment; - const capitalizedPath = component.charAt(0).toUpperCase() + component.slice(1); + // Convert kebab-case to Title Case (e.g., "button-group" → "Button Group") + const capitalizedPath = component + .split("-") + .map(word => word.charAt(0).toUpperCase() + word.slice(1)) + .join(" ");
- Consider adding more specific error messages when possible instead of just displaying the generic error:
addToast({ title: "Error", - description: error ?? "Unknown error", + description: error ? `Failed to open chat: ${error}` : "Failed to open chat due to an unknown error", color: "danger", });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/docs/.env.example
(1 hunks)apps/docs/actions/open-in-chat.ts
(1 hunks)apps/docs/app/docs/[[...slug]]/page.tsx
(2 hunks)apps/docs/components/docs/components/code-demo/code-demo.tsx
(3 hunks)apps/docs/components/docs/components/code-demo/parse-dependencies.ts
(1 hunks)
🔇 Additional comments (9)
apps/docs/app/docs/[[...slug]]/page.tsx (2)
5-5
: Import addition looks goodThe ToastProvider import from @heroui/react has been correctly added to support the new chat feature's error notifications.
107-108
: Conditional ToastProvider rendering implemented correctlyGood implementation to avoid duplicate ToastProvider on the Toast documentation page. The conditional check and comment provide clear context for why this approach was taken.
apps/docs/.env.example (1)
24-29
: Environment variables added correctly for chat featureThe new environment variables for the chat feature have been properly documented in the .env.example file with a clear section header. This will help other developers understand what environment variables they need to set up.
apps/docs/components/docs/components/code-demo/parse-dependencies.ts (1)
3-28
: Dependencies parsing logic looks solidThe function correctly filters out internal (@heroui) and relative imports while constructing a dependencies array with appropriate versions.
One minor suggestion: Consider adding type annotations for better clarity and documentation:
- export const parseDependencies = (content: string) => { + export const parseDependencies = (content: string): {name: string; version: string}[] => {apps/docs/actions/open-in-chat.ts (2)
21-28
: React import check logic is correctGood approach to ensure React is imported when React.* syntax is used in the code.
47-54
: Validate environment variablesThe code assumes process.env.CHAT_URL will be defined, but doesn't check for it.
Add a check to ensure environment variables are defined:
+ if (!process.env.CHAT_URL) { + return { + error: "CHAT_URL environment variable is not defined", + data: null, + }; + } + return {error: null, data: `${process.env.CHAT_URL}${result.path}`};apps/docs/components/docs/components/code-demo/code-demo.tsx (3)
3-8
: Well-structured imports, good organization.The new imports are organized logically, bringing in all the necessary functionality for the chat feature while maintaining clean code structure.
83-85
: Good setup of state and hooks.Proper use of PostHog for analytics tracking and useState for managing the loading state of the chat button.
233-253
: Good UI implementation of the chat button.The button is nicely integrated with loading state indicators. The conditional rendering based on the page type is appropriate.
One consideration:
The absolute positioning (
absolute right-0 top-1
) might overlap with other elements depending on the layout. Verify that this positioning works well across different viewport sizes and doesn't interfere with other UI elements in the header area.
} catch (error) { | ||
return {error: error, data: null}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve error handling in catch block
The error message may not be a string, which could cause issues when returned.
- return {error: error, data: null};
+ const errorMessage = error instanceof Error ? error.message : "Unknown error occurred";
+ console.error("Error in openInChat:", error);
+ return {error: errorMessage, data: null};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
} catch (error) { | |
return {error: error, data: null}; | |
} | |
} catch (error) { | |
const errorMessage = error instanceof Error ? error.message : "Unknown error occurred"; | |
console.error("Error in openInChat:", error); | |
return {error: errorMessage, data: null}; | |
} |
const response = await fetch(`${process.env.CHAT_API_URL}/import`, { | ||
method: "POST", | ||
headers: { | ||
"Content-Type": "application/json", | ||
Authorization: `Bearer ${process.env.IMPORT_API_KEY}`, | ||
}, | ||
body: JSON.stringify({ | ||
title, | ||
content, | ||
dependencies, | ||
}), | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for API request
The fetch request lacks timeout and network error handling.
- const response = await fetch(`${process.env.CHAT_API_URL}/import`, {
+ const controller = new AbortController();
+ const timeoutId = setTimeout(() => controller.abort(), 10000); // 10-second timeout
+
+ const response = await fetch(`${process.env.CHAT_API_URL}/import`, {
method: "POST",
headers: {
"Content-Type": "application/json",
Authorization: `Bearer ${process.env.IMPORT_API_KEY}`,
},
+ signal: controller.signal,
body: JSON.stringify({
title,
content,
dependencies,
}),
});
+
+ clearTimeout(timeoutId);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const response = await fetch(`${process.env.CHAT_API_URL}/import`, { | |
method: "POST", | |
headers: { | |
"Content-Type": "application/json", | |
Authorization: `Bearer ${process.env.IMPORT_API_KEY}`, | |
}, | |
body: JSON.stringify({ | |
title, | |
content, | |
dependencies, | |
}), | |
}); | |
const controller = new AbortController(); | |
const timeoutId = setTimeout(() => controller.abort(), 10000); // 10-second timeout | |
const response = await fetch(`${process.env.CHAT_API_URL}/import`, { | |
method: "POST", | |
headers: { | |
"Content-Type": "application/json", | |
Authorization: `Bearer ${process.env.IMPORT_API_KEY}`, | |
}, | |
signal: controller.signal, | |
body: JSON.stringify({ | |
title, | |
content, | |
dependencies, | |
}), | |
}); | |
clearTimeout(timeoutId); |
Closes #
open in heroui chat button for component code demos
parses dependencies from demo code to pass to api
the error handling toast needs a check for the toast docs page as it already has a provider
📝 Description
⛳️ Current behavior (updates)
🚀 New behavior
💣 Is this a breaking change (Yes/No):
📝 Additional Information
Summary by CodeRabbit