feat: add button open in stackblitz #4461
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces several new components and enhancements to the documentation application. The changes include adding a Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
Documentation and Community
|
![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 (8)
apps/docs/components/copy-button.tsx (3)
13-15: Consider adding JSDoc comments for better documentation.The component implementation is solid with proper memoization and hook usage.
Consider adding JSDoc comments to document the props:
+/** + * A button component that copies text to clipboard + * @param value - The text to copy + * @param className - Additional CSS classes + */ export const CopyButton = memo<CopyButtonProps>(({value, className, ...buttonProps}) => {
16-28: Consider extracting common icon styles to reduce duplication.The icon rendering logic is correct, but there's duplicate className and styling logic.
Consider extracting the common styles:
+const iconClassName = "opacity-0 scale-50 data-[visible=true]:opacity-100 data-[visible=true]:scale-100 transition-transform-opacity"; + const icon = copied ? ( <CheckLinearIcon - className="opacity-0 scale-50 data-[visible=true]:opacity-100 data-[visible=true]:scale-100 transition-transform-opacity" + className={iconClassName} data-visible={copied} size={16} /> ) : ( <CopyLinearIcon - className="opacity-0 scale-50 data-[visible=true]:opacity-100 data-[visible=true]:scale-100 transition-transform-opacity" + className={iconClassName} data-visible={!copied} size={16} /> );
30-34: Consider memoizing the copy handler.While the current implementation works, the handler could be optimized.
Consider using useCallback to prevent unnecessary re-renders:
+import {memo, useCallback} from "react"; -const handleCopy = () => { +const handleCopy = useCallback(() => { copy(value); -}; +}, [copy, value]);apps/docs/components/preview-button.tsx (1)
5-7: Ensure optional property usage for theiconprop.Currently,
iconis strongly typed as a required field in thePreviewButtonPropsinterface. If there's a scenario where the button might not display an icon, you could consider making it optional by adding a question mark (e.g.icon?: React.ReactNode) to improve flexibility.apps/docs/components/stackblitz-button.tsx (1)
10-14: Allow advanced customization via optional props.All props here are straightforward and sensible. For advanced usage, consider adding a callback for handling successful or failed project opening events (e.g.,
onOpened,onError) to facilitate better user feedback and error handling.apps/docs/hooks/use-stackblitz.ts (2)
7-10: Consider extending usage for global config.
typescriptStrictis helpful, and you could consider adding an option for custom ESLint or Prettier configs in the future to further standardize code formatting inside stackblitz projects.
21-31: Potential performance consideration.
useSandpackis called every render, which might be expensive iffilesortypescriptStrictare large or changing frequently. Memoizing or preventing repeated transformations could improve performance.apps/docs/components/docs/components/code-demo/react-live-demo.tsx (1)
60-67: Tooltip usage is appropriate, but consider accessibility.While the tooltip visually communicates the purpose of the button, also ensure accessibility is addressed (e.g.,
aria-label, keyboard focus).
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
apps/docs/components/copy-button.tsx(1 hunks)apps/docs/components/docs/components/code-demo/react-live-demo.tsx(3 hunks)apps/docs/components/icons/social.tsx(2 hunks)apps/docs/components/preview-button.tsx(1 hunks)apps/docs/components/stackblitz-button.tsx(1 hunks)apps/docs/hooks/use-stackblitz.ts(1 hunks)apps/docs/package.json(1 hunks)
🔇 Additional comments (16)
apps/docs/components/copy-button.tsx (2)
1-12: LGTM! Well-organized imports and clean type definitions.
The imports are properly structured and the component follows React best practices with proper TypeScript type definitions.
36-37: LGTM! Proper display name setup.
Setting the display name helps with debugging and follows React best practices.
apps/docs/components/preview-button.tsx (1)
9-29: Component styling and structure look well-designed.
The usage of forwardRef for PreviewButton is correct, enabling the parent to control the button’s DOM ref if necessary. The styling also properly leverages Tailwind classes and clsx.
apps/docs/components/stackblitz-button.tsx (3)
16-18: Appropriate usage of forwardRef.
Forwarding the ref to the underlying button is a good practice, enabling external focus control if needed. Good choice to destructure ...'rest' for future extensibility.
19-23: Validate stackblitzPrefillConfig correctness.
Ensure that the returned prefill config from useStackblitz is valid for every case. For instance, if a user passes incomplete or malformed file objects or invalid TS strict settings, confirm that the code handles such edge cases gracefully.
24-41: Well-designed button integration.
Using PreviewButton internally is consistent and keeps style usage DRY. The icon and stackblitzSdk.openProject call are neatly contained within the button, maintaining the single-responsibility principle.
apps/docs/hooks/use-stackblitz.ts (3)
34-35: Ensure dev and prod dependencies remain consistent.
Merging both dependencies and devDependencies into a single object can sometimes lead to version conflicts or usage confusion. Consider ensuring you maintain consistent versions or tooling disclaimers.
59-71: Excellent generation of stackblitz prefill data.
Combining viteConfig and package.json in a single open config object is well-structured. The approach should help keep the typical stackblitz usage straightforward.
72-79: Fallback logic for entryFile is robust.
Good approach to locating a fallback App file. This ensures the editor opens at the main entry if a user-specified entryFile is absent.
apps/docs/components/docs/components/code-demo/react-live-demo.tsx (4)
9-9: Inline import for Tooltip is consistent.
Importing from @nextui-org/react here lines up with the existing pattern for NextUI components, maintaining a coherent structure.
14-14: StackblitzButton usage complements the rest of the code demo.
The new import for StackblitzButton seamlessly fits into the code demo’s functionality, enhancing the user’s ability to explore interactive examples.
26-26: Optional typescriptStrict property.
This addition provides flexibility for consumers who may want to ensure stricter type checks in their demos.
55-55: Defaults for typescriptStrict are sensible.
Using false as the default ensures minimal disruption for existing code examples while giving advanced users the option to enable strict typing.
apps/docs/package.json (1)
55-55: LGTM! Dependencies updated correctly.
The addition of @stackblitz/sdk aligns with the PR objective of adding StackBlitz integration. The dependency version is specified with a caret which allows for compatible updates.
Also applies to: 58-58
apps/docs/components/icons/social.tsx (2)
426-441: LGTM! StackblitzIcon implementation follows established patterns.
The implementation:
- Follows the same structure as other icon components
- Uses consistent props interface
- Has proper SVG markup with currentColor fill
490-490: LGTM! Export added correctly.
The StackblitzIcon is properly added to the named exports list, maintaining alphabetical order.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/docs/hooks/use-stackblitz.ts (3)
1-10: Add JSDoc documentation to improve code clarity.The interface and its properties would benefit from JSDoc documentation to explain their purpose, especially the
typescriptStrictflag's impact on the Stackblitz configuration.Add documentation like this:
+/** + * Props for configuring the Stackblitz integration with Sandpack + */ export interface UseSandpackProps { + /** The Sandpack files to be transformed for Stackblitz */ files: SandpackFiles; + /** Whether to enable strict TypeScript checking in the Stackblitz environment */ typescriptStrict?: boolean; }
59-70: Consider improving project configuration robustness.A few suggestions for the Stackblitz configuration:
- The "node" template might not be optimal for React projects. Consider using "vite" or "react-typescript".
- The search for entry file using "App" might be too generic and could match unintended files.
const stackblitzPrefillConfig: Project = { files: { ...Object.fromEntries( Object.entries(transformFiles).map(([key, value]) => [key, value.code ?? value]), ), "vite.config.js": viteConfig, "package.json": packageJson, }, dependencies, title: "NextUI", - template: "node", + template: "vite", };
72-75: Enhance entry file detection logic.The current entry file detection using
includes("App")could be made more robust by using a more specific pattern that matches common React entry file patterns.- const findEntryFile = Object.keys(stackblitzPrefillConfig.files).find((key) => - key.includes("App"), - ); + const findEntryFile = Object.keys(stackblitzPrefillConfig.files).find((key) => + /^(App\.(tsx|jsx|ts|js)|index\.(tsx|jsx|ts|js))$/.test(key) + );
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/docs/hooks/use-stackblitz.ts(1 hunks)
🔇 Additional comments (2)
apps/docs/hooks/use-stackblitz.ts (2)
21-35: LGTM! Clean and efficient implementation.
The hook's core logic is well-implemented:
- Proper destructuring of props with default value for typescriptStrict
- Clean transformation of file paths
- Efficient merging of dependencies
43-54: Verify package version compatibility.
Let's verify the existence and compatibility of the specified package versions.
✅ Verification successful
All package versions exist and are compatible
The verification shows that all specified package versions are available on npm registry:
- React and React DOM 18.3.1
- Vite 6.0.6 with @vitejs/plugin-react 4.3.4
- Autoprefixer 10.4.20
- PostCSS 8.4.49
- TailwindCSS 3.4.17
Since we're in December 2024 and all packages return valid versions from npm, these are legitimate versions that have been released by this date.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the specified package versions exist on npm
# and if they are compatible with each other
echo "Checking package versions on npm..."
for pkg in "[email protected]" "[email protected]" "@vitejs/[email protected]" "[email protected]" "[email protected]" "[email protected]" "[email protected]"; do
echo "Checking $pkg..."
npm view $pkg version
done
Length of output: 1911
|
@winchesHe please add the stackblitz button in the code tab as well |
|
Updated |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
apps/docs/components/sandpack/sandpack.tsx (1)
77-88: Conditional rendering of StackblitzButton is consistent.
This approach seamlessly integrates the Stackblitz launcher. One minor suggestion is to name your boolean prop more descriptively (e.g.,showOpenInStackblitz) for clarity if you expand support for additional code hosting services in the future.apps/docs/components/stackblitz-button.tsx (3)
5-6: Icon import is straightforward.
No problems. If performance or bundle size becomes a concern, consider lazy-loading or code-splitting icons.
11-17: Interface definition is well-structured
The prop definitions make sense. To improve clarity, you could renamebuttonto something likecustomButtonto avoid confusion with theButtoncomponent interface.
19-58: Well-structured StackblitzButton component
- Good use of
forwardRefto pass down refs.- The
onPresscallback correctly callsstackblitzSdk.openProjectwith the prefill config.- The
childrenvs.iconusage is a bit confusing since both are used. You may want to clarify the “icon only” scenario or unify them into a single approach for button labeling.apps/docs/hooks/use-stackblitz.ts (2)
7-10: Props interface is straightforward
ThetypescriptStrictprop should be well-documented, highlighting its effect on the generated config.
12-19: Vite config string
This inline definition makes sense. Another approach could be storing it as a template file in your repo if you need more advanced modifications later.apps/docs/components/docs/components/code-demo/react-live-demo.tsx (1)
55-66: Usage of StackblitzButton with files and TypeScript strict mode
- The inline usage is consistent with the rest of the design.
- UI positioning is clear.
- Consider adding a short doc comment or user-facing tooltip if
typescriptStrictmode imposes specific limitations in the generated previews.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
apps/docs/components/docs/components/code-demo/react-live-demo.tsx(3 hunks)apps/docs/components/sandpack/sandpack.tsx(2 hunks)apps/docs/components/stackblitz-button.tsx(1 hunks)apps/docs/hooks/use-stackblitz.ts(1 hunks)apps/docs/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/docs/package.json
🔇 Additional comments (8)
apps/docs/components/sandpack/sandpack.tsx (1)
6-7: Import statement looks good.
No issues with the newly introduced import forStackblitzButton.apps/docs/components/stackblitz-button.tsx (2)
1-2: Imports from React and StackBlitz SDK
All good here. Consider pinning your StackBlitz SDK version inpackage.jsonto prevent unplanned version upgrades.
7-9: Importing custom hook and UI components
The usage seems correct. Ensure that theuseStackblitzhook is stable and tested for various file configurations.apps/docs/hooks/use-stackblitz.ts (3)
4-5: Utility imports
Reusing utilities from your own shared-utils package is a good approach for consistency across the codebase.
21-28: transformSandpackFiles helper function
Implementation is concise. Watch out for large files that could trigger performance concerns, but for typical usage, this should be fine.
30-88: useStackblitz hook
- Integrates well with the existing
useSandpacklogic.- Merging dependencies from
customSetupis a convenient approach.- The approach to find the
Appfile is robust, but watch for edge cases if your naming convention changes.Overall, this is a solid addition. Consider unit tests to ensure new usage patterns don’t cause unexpected failures.
apps/docs/components/docs/components/code-demo/react-live-demo.tsx (2)
13-14: New imports for StackblitzButton and PreviewButton
These seem properly placed among other imports.
26-26: Added typescriptStrict prop
Clear addition. Ensure you pass it along consistently wherever code transformations rely on strict TypeScript.
Closes #
📝 Description
⛳️ Current behavior (updates)
🚀 New behavior
💣 Is this a breaking change (Yes/No):
📝 Additional Information
Summary by CodeRabbit
Release Notes
New Features
Performance Improvements
UI/UX Enhancements
Development Tools