-
Notifications
You must be signed in to change notification settings - Fork 4
feat: Add TextField component with ReactNode label support #19
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: Add TextField component with ReactNode label support #19
Conversation
- Add TextField component to plugins SDK that accepts both string and ReactNode labels - Add @medusajs/ui and React as peer dependencies to plugins SDK - Export TextField component from SDK for use across plugins - Update webhook modal to use new TextField component as example - Maintain backward compatibility while providing enhanced flexibility
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant AdminModal as WebhookModal
participant Form as React Hook Form
participant TextField as TextField Component
AdminModal->>Form: Register "target_url" field
AdminModal->>TextField: Render TextField with props (label, error, etc.)
TextField-->>AdminModal: Renders input with label, error, helper text
AdminModal->>Form: Handles validation and submission
Estimated code review effort2 (~15 minutes) Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
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: 0
🧹 Nitpick comments (2)
packages/plugins-sdk/src/components/TextField.tsx (2)
51-51
: Use a more robust ID generation method.The current
Math.random()
approach could potentially create ID collisions in edge cases, especially with many component instances.Consider using a more robust approach:
- const inputId = id || React.useMemo(() => `textfield-${Math.random().toString(36).substr(2, 9)}`, []); + const inputId = id || React.useMemo(() => `textfield-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`, []);Or use a counter-based approach for guaranteed uniqueness.
61-65
: Consider adding accessibility attributes for better screen reader support.The component could benefit from additional ARIA attributes when in error state.
<Input id={inputId} + aria-invalid={!!error} + aria-describedby={error ? `${inputId}-error` : helperText ? `${inputId}-helper` : undefined} className={`${error ? 'border-ui-border-error' : ''} ${className}`} {...inputProps} />And update the error/helper text elements:
{error && ( - <span className="text-ui-fg-error text-sm">{error}</span> + <span id={`${inputId}-error`} className="text-ui-fg-error text-sm">{error}</span> )} {helperText && !error && ( - <span className="text-ui-fg-muted text-sm">{helperText}</span> + <span id={`${inputId}-helper`} className="text-ui-fg-muted text-sm">{helperText}</span> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/plugins-sdk/package.json
(1 hunks)packages/plugins-sdk/src/components/TextField.tsx
(1 hunks)packages/plugins-sdk/src/components/index.ts
(1 hunks)packages/plugins-sdk/src/index.ts
(1 hunks)plugins/webhooks/src/admin/modals/webhook-modal.tsx
(3 hunks)
🧠 Learnings (1)
packages/plugins-sdk/package.json (1)
Learnt from: antony-lambdacurry
PR: #5
File: plugins/page-builder/src/modules/page-builder/migrations/.snapshot-medusa-page-builder.json:1409-1416
Timestamp: 2025-03-06T23:30:43.152Z
Learning: The .snapshot-medusa-page-builder.json
file is auto-generated by Medusa.js and should not be manually modified.
🧰 Additional context used
🧠 Learnings (1)
packages/plugins-sdk/package.json (1)
Learnt from: antony-lambdacurry
PR: #5
File: plugins/page-builder/src/modules/page-builder/migrations/.snapshot-medusa-page-builder.json:1409-1416
Timestamp: 2025-03-06T23:30:43.152Z
Learning: The .snapshot-medusa-page-builder.json
file is auto-generated by Medusa.js and should not be manually modified.
🔇 Additional comments (7)
packages/plugins-sdk/package.json (1)
54-58
: LGTM! Peer dependencies properly configured for the new TextField component.The peer dependencies correctly specify the required packages and versions for the new React TextField component. The version ranges are appropriate and provide good compatibility coverage.
packages/plugins-sdk/src/index.ts (1)
3-3
: LGTM! Export correctly extends the public API.The new export follows the established pattern and properly makes the TextField component available to package consumers.
packages/plugins-sdk/src/components/index.ts (1)
1-1
: LGTM! Component exports are properly structured.The export uses correct TypeScript syntax with the
type
keyword for the interface and follows best practices for module exports.packages/plugins-sdk/src/components/TextField.tsx (1)
4-33
: Excellent component implementation with good TypeScript support and flexibility.The TextField component is well-designed with:
- Proper TypeScript typing and JSDoc documentation
- Flexible label support (string or ReactNode)
- Good error handling and styling
- Proper accessibility with label association
- Clean separation of concerns
The component follows React and @medusajs/ui patterns effectively.
Also applies to: 39-74
plugins/webhooks/src/admin/modals/webhook-modal.tsx (3)
13-13
: LGTM! Clean import of the new TextField component.The import correctly brings in the TextField component from the plugins SDK.
59-59
: Good addition of errors destructuring for TextField integration.The errors destructuring from formState is correctly added to support error display in the TextField component.
176-190
: Excellent TextField integration that simplifies the form structure.The TextField usage is well-implemented with:
- Proper integration with react-hook-form via
register
- Comprehensive URL validation pattern
- Clean error handling via
errors.target_url?.message
- Appropriate field configuration (type, placeholder, required)
- Maintains all existing validation logic while simplifying the JSX
This replacement successfully consolidates label, input, and error display into a single reusable component.
Summary
This PR adds a new
TextField
component to the plugins SDK that supports both string and ReactNode labels, providing enhanced flexibility for form inputs across all plugins.Changes Made
🆕 New TextField Component
packages/plugins-sdk/src/components/TextField.tsx
label
prop as eitherstring
orReactNode
@medusajs/ui
Input and Label components📦 Dependencies & Exports
@medusajs/ui
,react
, and@types/react
as peer dependencies to plugins SDKpackages/plugins-sdk/src/components/index.ts
🔧 Example Implementation
plugins/webhooks/src/admin/modals/webhook-modal.tsx
) to use the new TextFieldUsage Examples
Basic String Label
ReactNode Label (with icons, formatting, etc.)
Benefits
✅ Flexibility: Supports both simple string labels and complex ReactNode labels
✅ Consistency: Maintains @medusajs/ui design patterns and styling
✅ Reusability: Available across all plugins through the shared SDK
✅ Type Safety: Full TypeScript support with proper prop forwarding
✅ Accessibility: Proper label association and ARIA attributes
Testing
The TextField component has been tested in the webhooks plugin modal and works correctly with:
Requested by: Mohsen Ghaemmaghami
💻 View my work • 🚫 Ban all checks • About Codegen
Summary by CodeRabbit
New Features
Improvements
Chores