feat(modal): enhance modal component with multiple animation variants #81
Conversation
Fixes #65 Add support for multiple animation variants in the Modal component. * Extend the `modalVariants` object in `packages/ui/src/components/modal.tsx` to include multiple animation options such as `fade`, `zoom`, `scaleBounce`, `slideUp`, `slideDown`, `slideRight`, `slideLeft`, `flip`, and `rotate`. * Allow dynamic selection of animation variants based on the `animationVariant` prop in the `Modal` component. * Update the documentation in `apps/www/content/docs/components/modal.mdx` to explain how to select and use the new animation variants. * Add test cases in `apps/www/app/playground/page.tsx` to cover all supported animation options for the `Modal` component. * Add a new file `apps/www/components/preview/Modal/modalAnimation.tsx` to demonstrate the use of different animation variants in the Modal component. * Update the version in `packages/ui/package.json` from `2.2.3` to `2.2.4`. --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/ruru-m07/ruru-ui/issues/65?shareId=XXXX-XXXX-XXXX-XXXX).
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
WalkthroughThe changes introduced in this pull request enhance the Changes
Assessment against linked issues
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
|
This pull request enhances the Enhancements to
Documentation updates:
|
ruru-m07
added
documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (7)
apps/www/components/preview/Modal/modalAnimation.tsx (1)
49-69: LGTM: Modal structure and content are well-implemented.The Modal component is correctly structured and uses the
selectedVariantappropriately for theanimationVariantprop. The content is well-organized with a clear header, input field, and action buttons.To enhance accessibility, consider adding an
aria-labelto the Modal component:- <Modal animationVariant={selectedVariant as keyof typeof modalVariants}> + <Modal animationVariant={selectedVariant as keyof typeof modalVariants} aria-label="Create Username Modal">This will provide better context for screen readers and improve the overall accessibility of your component.
apps/www/public/registry/components/modal.json (1)
Line range hint
62-242: LGTM: ComprehensivemodalVariantsobject with suggestion for 3D animationsThe
modalVariantsobject is well-implemented, providing a diverse range of animation options that align perfectly with the PR objectives. Each variant is clearly defined with appropriate properties for hidden, visible, and exit states.Suggestion for enhancement:
Consider adding theperspectiveproperty to 3D animations likeflipandrotatefor a more polished effect. For example:flip: { hidden: { opacity: 0, rotateY: "90deg", perspective: 1000, transition: { duration: 0.15 }, }, // ... (apply to visible and exit states as well) },This addition can enhance the depth perception of these 3D animations.
packages/ui/src/components/modal.tsx (3)
62-68: LGTM! Consider enhancing the documentation.The addition of the
animationVariantprop to theModalPropsinterface is well-implemented. It provides type safety by usingkeyof typeof modalVariantsand maintains backward compatibility with the default value.Consider adding an example of usage in the JSDoc comment to make it more clear for developers. For instance:
/** * The animation variant to use for the modal. * * @type {keyof typeof modalVariants} * @default "default" * @example * <Modal animationVariant="zoom"> * {/* Modal content *\/} * </Modal> */ animationVariant?: keyof typeof modalVariants;
130-304: Great addition of animation variants! Consider minor adjustments for consistency.The
modalVariantsconstant provides a rich set of animation options, enhancing the flexibility of the Modal component. The animations are well-defined and cover a good range of effects.For consistency, consider the following adjustments:
The
scaleBouncevariant uses a duration of 0.3s for thevisiblestate, while all other variants use 0.15s. Consider either:
a) UpdatingscaleBounceto use 0.15s for consistency, or
b) Adding a comment explaining why this animation has a different duration.Consider extracting the common transition duration into a constant for easier maintenance:
const DEFAULT_TRANSITION_DURATION = 0.15; // Then use it in the variants like this: transition: { duration: DEFAULT_TRANSITION_DURATION },This would make it easier to adjust the duration globally if needed in the future.
363-363: LGTM! Consider adding error handling for invalid animation variants.The implementation of the
animationVariantprop in theModalcomponent is correct and maintains backward compatibility with the default value.Consider adding a runtime check for invalid animation variants. While TypeScript provides compile-time safety, it's good practice to handle potential runtime errors:
const selectedVariant = modalVariants[animationVariant] || modalVariants.default; <motion.div // ...other props variants={selectedVariant} // ...other props > {children} </motion.div>This ensures that even if an invalid variant is somehow passed at runtime, the modal will still function with the default animation.
Also applies to: 398-401
apps/www/content/docs/components/modal.mdx (2)
528-614: LGTM: New Animation Variants sectionThe new Animation Variants section effectively demonstrates the added support for various animation options, aligning well with the PR objectives. The example using the
ModalAnimationcomponent is comprehensive and illustrates how to use the newanimationVariantprop.Consider adding a brief explanation of each animation variant (e.g., fade, zoom, scaleBounce, etc.) to help users understand what each option does visually. This could be added as a short list or table before the code example.
Line range hint
1-651: Overall assessment: Documentation updates align well with PR objectivesThe changes made to the Modal component documentation effectively capture the new animation variants feature. The addition of the Animation Variants section with a comprehensive example, along with the update to the props table, provides users with the necessary information to utilize the new functionality.
These updates align well with the PR objectives of enhancing the Modal component with multiple animation variants. The documentation now reflects the added support for various animation options and guides users on how to implement them.
To further improve the documentation:
- Consider adding brief descriptions of each animation variant to help users visualize the effects.
- Ensure that all supported animation variants are accurately listed in the props table.
- If possible, consider adding visual examples (e.g., GIFs) of each animation variant to enhance user understanding.
These enhancements will provide a more comprehensive and user-friendly documentation for the updated Modal component.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- apps/www/app/playground/page.tsx (2 hunks)
- apps/www/components/preview/Modal/modalAnimation.tsx (1 hunks)
- apps/www/content/docs/components/modal.mdx (2 hunks)
- apps/www/public/registry/components/modal.json (1 hunks)
- packages/ui/package.json (1 hunks)
- packages/ui/src/components/modal.tsx (4 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/ui/package.json
🧰 Additional context used
🪛 Biome
apps/www/components/preview/Modal/modalAnimation.tsx
[error] 44-44: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (8)
apps/www/components/preview/Modal/modalAnimation.tsx (3)
1-17: LGTM: Imports and dependencies are well-organized.The imports are correctly structured and include all necessary dependencies for the component. The "use client" directive is appropriately placed at the top of the file, indicating that this is a Client Component in Next.js.
74-74: LGTM: Component export is correct.The
ModalAnimationcomponent is properly exported as the default export, which is a common and acceptable practice for React components.
1-74: Overall, great implementation of the ModalAnimation component.This new component effectively demonstrates the enhanced Modal functionality with multiple animation variants, aligning well with the PR objectives. The code is well-structured, uses appropriate UI components, and provides a user-friendly interface for selecting and demonstrating different animation options.
A few minor improvements were suggested:
- Enhancing the
handleSubmitfunction with better error handling and state management.- Adding a
keyprop to the mappedSelectItemcomponents for better React performance.- Including an
aria-labelon the Modal component for improved accessibility.These small changes will further improve the component's functionality, performance, and accessibility. Great work on implementing this feature!
🧰 Tools
🪛 Biome
[error] 44-44: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
apps/www/public/registry/components/modal.json (3)
Line range hint
36-36: LGTM: NewanimationVariantproperty aligns with PR objectivesThe addition of the
animationVariantproperty to theModalPropsinterface is a well-implemented change that directly supports the PR's goal of enhancing the Modal component with multiple animation variants. The typingkeyof typeof modalVariantsensures type safety by only allowing valid animation variants to be used.
Line range hint
293-307: LGTM: Correct implementation ofanimationVariantin Modal componentThe changes in the Modal component implementation correctly utilize the new
animationVariantprop:
- The prop is added with a default value of "default", ensuring backward compatibility.
- The
variantsprop of the innermotion.divnow usesmodalVariants[animationVariant], allowing for dynamic selection of animation variants.These changes align perfectly with the PR objectives, providing users with the ability to choose from various animation options when opening or closing the modal.
Line range hint
1-482: LGTM: Successful implementation of multiple animation variants for Modal componentThis review confirms that the changes in the
modal.tsxcontent within themodal.jsonfile successfully implement the enhancement of the Modal component with multiple animation variants, as outlined in the PR objectives and issue #65. Key points:
- The
animationVariantprop is added toModalPropswith proper typing.- A comprehensive
modalVariantsobject is implemented with various animation options.- The Modal component is updated to utilize the new
animationVariantprop.These changes provide users with the ability to choose from a variety of animations when opening or closing the modal, increasing flexibility and customization options as intended. The implementation is type-safe, maintains consistency with the existing codebase, and ensures backward compatibility.
packages/ui/src/components/modal.tsx (1)
Line range hint
1-605: Overall, excellent enhancement of the Modal component!The implementation of multiple animation variants for the Modal component is well-executed and aligns perfectly with the PR objectives. The changes provide users with a rich set of animation options while maintaining backward compatibility.
Key points:
- The
ModalPropsinterface is correctly updated with the newanimationVariantprop.- The
modalVariantsconstant provides a comprehensive set of animation options.- The
Modalcomponent correctly implements the new animation feature.The suggestions provided are minor and aimed at improving documentation, consistency, and error handling. Great job on this enhancement!
apps/www/content/docs/components/modal.mdx (1)
17-17: LGTM: New import statement for ModalAnimationThe import statement for
ModalAnimationis correctly formatted and follows the existing import pattern. This import is necessary for the new Animation Variants section.
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (5)
apps/www/app/playground/page.tsx (1)
613-642: LGTM: Effective implementation of multiple modal variantsThe implementation successfully demonstrates all available modal variants using
Object.keys(modalVariants). Each variant is properly rendered with a unique key and the correctanimationVariantprop. The consistent content across modals allows for easy comparison of animation effects.To improve readability, consider extracting the modal content into a separate component:
const ModalContent = () => ( <Modal.Body> <Modal.Header> <Modal.Title>Create Username</Modal.Title> <Modal.Subtitle> Enter a unique name for your token to differentiate it from other tokens and then select the scope. </Modal.Subtitle> </Modal.Header> <Modal.Content> <Input label="username" placeholder="enter your username." /> </Modal.Content> </Modal.Body> ); // Then in the map function: <Modal animationVariant={variantKey}> <ModalContent /> <Modal.Actions> <Modal.Close variant="secondary">Cancel</Modal.Close> <Modal.Action onClick={handleSubmit}>Submit</Modal.Action> </Modal.Actions> </Modal>This refactoring would reduce repetition and make the code more maintainable.
apps/www/content/docs/components/modal.mdx (2)
528-614: LGTM: Comprehensive documentation for Animation VariantsThe new "Animation Variants" section provides a clear and detailed explanation of the new feature. The inclusion of both a preview and a code example is excellent for helping users understand and implement the feature. The interactive demonstration using the Select component is particularly helpful.
Consider adding a brief description of each animation variant to help users choose the most appropriate one for their use case. This could be added as a list or table before the code example.
622-626: LGTM: New animationVariant prop added to the props tableThe
animationVariantprop has been correctly added to the props table with an accurate type definition and a clear description. This addition properly documents the new feature for users.There's a minor formatting issue in the table. The pipe characters (
|) at the beginning and end of each line are misaligned. Please adjust the spacing to ensure all pipe characters are vertically aligned for better readability.apps/www/components/preview/Modal/modalAnimation.tsx (2)
74-74: Standardize capitalization in labels and placeholdersEnsure consistent capitalization in the label and placeholder text. Consider capitalizing "Username" and starting the placeholder with a capital letter.
Apply this diff:
-<Input label="username" placeholder="enter your username." /> +<Input label="Username" placeholder="Enter your username." />
85-85: Remove unnecessary whitespace in JSXThe
{" "}between theSpinnerand "Submit" is unnecessary since themr-2class adds sufficient spacing.Apply this diff:
-{submissionStatus === "loading" && <Spinner className="mr-2" />}{" "} Submit +{submissionStatus === "loading" && <Spinner className="mr-2" />}Submit
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- apps/www/app/playground/page.tsx (2 hunks)
- apps/www/components/preview/Modal/modalAnimation.tsx (1 hunks)
- apps/www/content/docs/components/modal.mdx (2 hunks)
- apps/www/public/registry/components/modal.json (1 hunks)
🧰 Additional context used
🔇 Additional comments (5)
apps/www/public/registry/components/modal.json (2)
6-6: Excellent addition of theanimationVariantprop!The new
animationVariantproperty in theModalPropsinterface is a great addition. It allows users to specify different animation styles for the modal, which aligns perfectly with the PR objective of enhancing the Modal component with multiple animation variants. The typekeyof typeof modalVariantsensures type safety by only allowing valid animation variant names to be used.
6-6: Summary: Excellent enhancement of the Modal component with animation variants!This implementation successfully achieves the PR objective of adding support for multiple animation variants to the Modal component. The changes include:
- Addition of the
animationVariantprop toModalProps.- Implementation of a comprehensive
modalVariantsobject with diverse animation options.- Integration of the new animation system into the Modal component.
These enhancements significantly improve the flexibility and customization options for the Modal component, allowing users to easily apply different animations. The implementation is clean, type-safe, and maintains backward compatibility.
Great job on this feature addition! The minor suggestions provided earlier can further refine this already solid implementation.
apps/www/app/playground/page.tsx (2)
41-41: LGTM: Proper import ofmodalVariantsThe import statement for
modalVariantsis correctly placed with other imports from the same module, which is a good practice for code organization.
Line range hint
1-643: Overall: Well-implemented modal variants demonstrationThe changes in this file effectively implement and showcase multiple modal variants. The code is well-structured and follows good practices. The suggested refactoring for the modal content is a minor improvement that could enhance readability and maintainability.
Great job on implementing this feature!
apps/www/content/docs/components/modal.mdx (1)
17-17: LGTM: New import statement added correctlyThe new import statement for
ModalAnimationis correctly placed and follows the existing import style. This is a good practice for importing components that will be used in the documentation.
…onent Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Fixes #65
Add support for multiple animation variants in the Modal component.
modalVariantsobject inpackages/ui/src/components/modal.tsxto include multiple animation options such asfade,zoom,scaleBounce,slideUp,slideDown,slideRight,slideLeft,flip, androtate.animationVariantprop in theModalcomponent.apps/www/content/docs/components/modal.mdxto explain how to select and use the new animation variants.apps/www/app/playground/page.tsxto cover all supported animation options for theModalcomponent.apps/www/components/preview/Modal/modalAnimation.tsxto demonstrate the use of different animation variants in the Modal component.packages/ui/package.jsonfrom2.2.3to2.2.4.For more details, open the Copilot Workspace session.
Summary by CodeRabbit
Release Notes
New Features
ModalAnimationcomponent for selecting animation styles.Documentation
Bug Fixes
Chores
ruru-uipackage version to 2.2.4.