-
Notifications
You must be signed in to change notification settings - Fork 0
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(TextInput): adding size prop #693
Conversation
WalkthroughThe pull request introduces a new Changes
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
|
Bundle Size (components)
Overall bundle size: 84.29 KB (+1 B 0.00%) Bundle Size (fingerprint)
Overall bundle size: 48.28 KB Bundle Size (form components)
Overall bundle size: 49.99 KB (+91 B +0.18%) Bundle Size (system)
Overall bundle size: 50.35 KB (+1 B 0.00%) |
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
🧹 Outside diff range and nitpick comments (2)
packages/documentation/src/Form/TextInput.stories.tsx (1)
34-37
: LGTM: Size options added to argTypes with appropriate control.The addition of the
size
property toargTypes
with options "xs", "sm", "md", "lg", and "xl" provides a good range of size variants. The use of a radio control type is appropriate for this selection.Consider adding a brief description for each size option to help users understand the visual differences:
size: { options: ["xs", "sm", "md", "lg", "xl"], control: { type: "radio" }, + description: "Size of the TextInput: xs (extra small), sm (small), md (medium), lg (large), xl (extra large)", },
packages/ui-textinput/src/components/TextInput/TextInputTypes.d.ts (1)
54-57
: Enhance JSDoc comment for thesize
property.The
size
property is well-defined and correctly typed. However, the JSDoc comment could be more specific about the default value.Consider updating the JSDoc comment to explicitly mention the default value:
/** - * Controls input height and horizontal padding, 'md' by default + * Controls input height and horizontal padding. + * @default "md" */ size?: Size;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (5)
- packages/documentation/src/Form/TextInput.stories.tsx (2 hunks)
- packages/ui-textinput/src/components/TextInput/TextInput.tsx (2 hunks)
- packages/ui-textinput/src/components/TextInput/TextInputTypes.d.ts (4 hunks)
- packages/ui-textinput/src/components/TextInput/tests/TextInput.test.tsx (1 hunks)
- packages/ui-textinput/src/components/TextInput/utilities.ts (4 hunks)
🔇 Additional comments (14)
packages/documentation/src/Form/TextInput.stories.tsx (2)
23-23
: LGTM: Size property added with appropriate default value.The addition of the
size
property with a default value of "md" aligns well with the PR objective. This provides a good starting point for users to customize the TextInput component size.
Line range hint
1-68
: Overall assessment: Changes are well-implemented and align with PR objectives.The addition of the
size
property to both the default export object and theargTypes
object provides a comprehensive setup for testing and demonstrating the new size variants of the TextInput component. The implementation is consistent and follows best practices for Storybook story definitions.packages/ui-textinput/src/components/TextInput/TextInputTypes.d.ts (4)
3-3
: LGTM: NewSize
type is well-defined.The
Size
type definition is clear, concise, and provides a good range of size options. It follows TypeScript best practices for defining union types.
59-59
: LGTM: Proper handling ofsize
property collision.The modification to omit the
size
property fromReact.InputHTMLAttributes<HTMLInputElement>
is a good practice. It prevents conflicts between the customsize
property and the native HTMLsize
attribute, ensuring type safety and avoiding potential runtime issues.
68-68
: LGTM: Simplified type definitions forTextInputProps
andTextInputMaskProps
.The modifications to
TextInputProps
andTextInputMaskProps
interfaces are appropriate. By extending onlyCommonTextInputProps
, the type definitions are simplified and redundancy is reduced. This change is consistent with the modification made toCommonTextInputProps
and maintains type safety.Also applies to: 86-86
Line range hint
1-86
: Overall assessment: Well-implemented changes with minor improvement suggested.The introduction of the
Size
type and its integration into theTextInput
component's type definitions are well-implemented. The changes follow TypeScript best practices and maintain type safety. The only suggestion is a minor improvement to the JSDoc comment for thesize
property. These changes provide a solid foundation for implementing size variants in theTextInput
component.packages/ui-textinput/src/components/TextInput/TextInput.tsx (3)
33-33
: LGTM: Addition ofsize
propThe
size
prop has been correctly added with a default value of "md". This aligns with the PR objectives and enhances the component's flexibility.
Line range hint
1-138
: Suggestion: Update tests and documentationThe changes to add the
size
prop look good overall. However, to ensure completeness:
- Please verify that the component's test suite has been updated to include tests for the new
size
prop and its various values.- Ensure that the component's documentation (if any) has been updated to reflect this new prop and its usage.
Run the following script to check for potential test and documentation files:
#!/bin/bash # Description: Check for test and documentation files related to TextInput # Test: Search for test files echo "Potential test files:" fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts TextInput # Test: Search for documentation files echo "Potential documentation files:" fd -e md -e mdx TextInput
53-53
: VerifygetTextInputClasses
implementationThe
size
prop is correctly passed to thegetTextInputClasses
function. This is necessary for applying size-specific styles.Please ensure that the
getTextInputClasses
function in the utilities file has been updated to handle the newsize
prop. Run the following script to verify:packages/ui-textinput/src/components/TextInput/utilities.ts (4)
10-10
: LGTM: Import of Size typeThe import of the
Size
type from./TextInputTypes
is correctly added to support the newsize
property ingetTextInputClassesProps
.
19-19
: LGTM: Addition of size propertyThe
size
property of typeSize
has been correctly added to thegetTextInputClassesProps
type. This addition aligns with the PR objective of adding a size prop to the TextInput component.
156-156
: LGTM: Addition of size parameterThe
size
parameter has been correctly added to the destructured parameters of thegetTextInputClasses
function. This change is consistent with the newsize
property ingetTextInputClassesProps
and aligns with the PR objective.
167-193
: LGTM: Implementation of size-based class assignmentThe implementation of size-based class assignment is well-structured and comprehensive:
- A switch statement correctly handles all possible size values: "xs", "sm", "md" (default), "lg", and "xl".
- Each size is assigned appropriate height and padding classes.
- The default case corresponds to the "md" size, which is consistent with the PR objective.
- The
sizeClass
is correctly incorporated into theinput
class string.This implementation effectively fulfills the requirement of adding a size prop to the TextInput component.
packages/ui-textinput/src/components/TextInput/__tests__/TextInput.test.tsx (1)
13-79
: LGTM! Comprehensive test suite for TextInput sizes.The new test suite thoroughly covers all size variants of the TextInput component. The use of parameterized testing with
it.each
is an excellent approach for testing multiple scenarios efficiently.A few suggestions for improvement:
Consider using an object map instead of a switch statement for determining height and padding classes. This can make the code more maintainable and easier to extend in the future.
Add a test case for the default size behavior when no size prop is provided.
Here's a suggested refactor for the size-specific logic:
const sizeClasses = { xs: { height: "h-8", padding: "px-2" }, sm: { height: "h-10", padding: "px-3" }, md: { height: "h-12", padding: "px-4" }, lg: { height: "h-14", padding: "px-4" }, xl: { height: "h-16", padding: "px-4" }, }; const { height: heightClass, padding: paddingClass } = sizeClasses[size] || sizeClasses.md;This approach eliminates the need for a switch statement and makes it easier to add or modify size variants in the future.
To ensure we're not missing any size variants, let's verify the TextInput component's prop types:
🚀 Automated Release --- <details><summary>ui-anchor: 1.1.5</summary> ## [1.1.5](ui-anchor-v1.1.4...ui-anchor-v1.1.5) (2024-09-26) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-button bumped to 1.1.5 * devDependencies * @versini/ui-styles bumped to 1.10.2 </details> <details><summary>ui-bubble: 1.0.5</summary> ## [1.0.5](ui-bubble-v1.0.4...ui-bubble-v1.0.5) (2024-09-26) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-button bumped to 1.1.5 * @versini/ui-icons bumped to 1.12.6 * @versini/ui-private bumped to 1.4.14 * devDependencies * @versini/ui-styles bumped to 1.10.2 </details> <details><summary>ui-button: 1.1.5</summary> ## [1.1.5](ui-button-v1.1.4...ui-button-v1.1.5) (2024-09-26) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.14 * devDependencies * @versini/ui-icons bumped to 1.12.6 * @versini/ui-styles bumped to 1.10.2 </details> <details><summary>ui-card: 1.0.5</summary> ## [1.0.5](ui-card-v1.0.4...ui-card-v1.0.5) (2024-09-26) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.14 * devDependencies * @versini/ui-styles bumped to 1.10.2 </details> <details><summary>ui-components: 5.31.6</summary> ## [5.31.6](ui-components-v5.31.5...ui-components-v5.31.6) (2024-09-26) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-anchor bumped to 1.1.5 * @versini/ui-button bumped to 1.1.5 * @versini/ui-bubble bumped to 1.0.5 * @versini/ui-card bumped to 1.0.5 * @versini/ui-footer bumped to 1.0.5 * @versini/ui-header bumped to 1.0.5 * @versini/ui-icons bumped to 1.12.6 * @versini/ui-main bumped to 1.0.5 * @versini/ui-menu bumped to 1.0.5 * @versini/ui-panel bumped to 1.0.5 * @versini/ui-pill bumped to 1.0.5 * @versini/ui-private bumped to 1.4.14 * @versini/ui-spinner bumped to 1.0.5 * @versini/ui-table bumped to 1.0.5 * devDependencies * @versini/ui-styles bumped to 1.10.2 </details> <details><summary>ui-footer: 1.0.5</summary> ## [1.0.5](ui-footer-v1.0.4...ui-footer-v1.0.5) (2024-09-26) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.14 * devDependencies * @versini/ui-styles bumped to 1.10.2 </details> <details><summary>ui-form: 1.6.5</summary> ## [1.6.5](ui-form-v1.6.4...ui-form-v1.6.5) (2024-09-26) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-textarea bumped to 1.0.4 * @versini/ui-textinput bumped to 1.1.0 * @versini/ui-toggle bumped to 1.0.4 * @versini/ui-private bumped to 1.4.14 * devDependencies * @versini/ui-button bumped to 1.1.5 * @versini/ui-icons bumped to 1.12.6 * @versini/ui-styles bumped to 1.10.2 </details> <details><summary>ui-header: 1.0.5</summary> ## [1.0.5](ui-header-v1.0.4...ui-header-v1.0.5) (2024-09-26) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.14 * devDependencies * @versini/ui-styles bumped to 1.10.2 </details> <details><summary>ui-icons: 1.12.6</summary> ## [1.12.6](ui-icons-v1.12.5...ui-icons-v1.12.6) (2024-09-26) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.14 * devDependencies * @versini/ui-private bumped to 1.4.14 </details> <details><summary>ui-main: 1.0.5</summary> ## [1.0.5](ui-main-v1.0.4...ui-main-v1.0.5) (2024-09-26) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.14 * devDependencies * @versini/ui-styles bumped to 1.10.2 </details> <details><summary>ui-menu: 1.0.5</summary> ## [1.0.5](ui-menu-v1.0.4...ui-menu-v1.0.5) (2024-09-26) ### Dependencies * The following workspace dependencies were updated * devDependencies * @versini/ui-button bumped to 1.1.5 * @versini/ui-icons bumped to 1.12.6 * @versini/ui-styles bumped to 1.10.2 </details> <details><summary>ui-panel: 1.0.5</summary> ## [1.0.5](ui-panel-v1.0.4...ui-panel-v1.0.5) (2024-09-26) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-button bumped to 1.1.5 * @versini/ui-icons bumped to 1.12.6 * @versini/ui-private bumped to 1.4.14 * devDependencies * @versini/ui-styles bumped to 1.10.2 </details> <details><summary>ui-pill: 1.0.5</summary> ## [1.0.5](ui-pill-v1.0.4...ui-pill-v1.0.5) (2024-09-26) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.14 * devDependencies * @versini/ui-styles bumped to 1.10.2 </details> <details><summary>ui-private: 1.4.14</summary> ## [1.4.14](ui-private-v1.4.13...ui-private-v1.4.14) (2024-09-26) ### Dependencies * The following workspace dependencies were updated * devDependencies * @versini/ui-styles bumped to 1.10.2 </details> <details><summary>ui-spinner: 1.0.5</summary> ## [1.0.5](ui-spinner-v1.0.4...ui-spinner-v1.0.5) (2024-09-26) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.14 * devDependencies * @versini/ui-styles bumped to 1.10.2 </details> <details><summary>ui-styles: 1.10.2</summary> ## [1.10.2](ui-styles-v1.10.1...ui-styles-v1.10.2) (2024-09-26) ### Bug Fixes * **TextInput:** misaligned label and helper text at different sizes ([#695](#695)) ([9ff6fa1](9ff6fa1)) </details> <details><summary>ui-system: 1.4.15</summary> ## [1.4.15](ui-system-v1.4.14...ui-system-v1.4.15) (2024-09-26) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.14 * devDependencies * @versini/ui-button bumped to 1.1.5 * @versini/ui-styles bumped to 1.10.2 </details> <details><summary>ui-table: 1.0.5</summary> ## [1.0.5](ui-table-v1.0.4...ui-table-v1.0.5) (2024-09-26) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-button bumped to 1.1.5 * @versini/ui-icons bumped to 1.12.6 * @versini/ui-private bumped to 1.4.14 * devDependencies * @versini/ui-styles bumped to 1.10.2 </details> <details><summary>ui-textarea: 1.0.4</summary> ## [1.0.4](ui-textarea-v1.0.3...ui-textarea-v1.0.4) (2024-09-26) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.14 * devDependencies * @versini/ui-styles bumped to 1.10.2 </details> <details><summary>ui-textinput: 1.1.0</summary> ## [1.1.0](ui-textinput-v1.0.3...ui-textinput-v1.1.0) (2024-09-26) ### Features * **TextInput:** adding size prop ([#693](#693)) ([397d22f](397d22f)) ### Bug Fixes * **TextInput:** misaligned label and helper text at different sizes ([#695](#695)) ([9ff6fa1](9ff6fa1)) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.14 * devDependencies * @versini/ui-button bumped to 1.1.5 * @versini/ui-icons bumped to 1.12.6 * @versini/ui-styles bumped to 1.10.2 </details> <details><summary>ui-toggle: 1.0.4</summary> ## [1.0.4](ui-toggle-v1.0.3...ui-toggle-v1.0.4) (2024-09-26) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-private bumped to 1.4.14 * devDependencies * @versini/ui-styles bumped to 1.10.2 </details> <details><summary>ui-truncate: 1.0.2</summary> ## [1.0.2](ui-truncate-v1.0.1...ui-truncate-v1.0.2) (2024-09-26) ### Dependencies * The following workspace dependencies were updated * dependencies * @versini/ui-button bumped to 1.1.5 * devDependencies * @versini/ui-styles bumped to 1.10.2 </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: aversini <[email protected]>
Summary by CodeRabbit
New Features
size
property for theTextInput
component, allowing users to select from multiple size options: "xs," "sm," "md," "lg," and "xl."Bug Fixes
TextInput
component to ensure proper styling based on selected size.Documentation
size
property and its options.