fix: remove disableAnimation prop from span element instead of img el… #3593
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
|
Important Review skippedAuto reviews are limited to specific labels. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe recent changes to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AvatarComponent
participant useAvatar
User->>AvatarComponent: Render Avatar
AvatarComponent->>useAvatar: Call useAvatar(originalProps)
useAvatar->>useAvatar: Initialize isDisableAnimation
useAvatar->>AvatarComponent: Return props
AvatarComponent->>DOM: Render with filtered props
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![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
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- packages/components/avatar/src/use-avatar.ts (6 hunks)
Additional comments not posted (5)
packages/components/avatar/src/use-avatar.ts (5)
120-120: Initialization ofdisableAnimationproperty.The
disableAnimationproperty is correctly initialized with a default value offalseand destructured fromoriginalProps, ensuring it is not passed to the<span>element.
139-139: Assignment ofisDisableAnimationvariable.The
isDisableAnimationvariable is correctly assigned the value ofdisableAnimationor the global context'sdisableAnimationif it exists, enhancing clarity and maintainability.
163-163: Usage ofisDisableAnimationinslotsobject.The
disableAnimationproperty in theslotsobject is correctly updated to useisDisableAnimation, ensuring accurate component behavior.
172-172: Addition ofisDisableAnimationto dependency array.The
isDisableAnimationvariable is correctly added to the dependency array of theuseMemohook, ensuring proper reactivity of the component.
205-207: Merging ofimgPropsandpropsingetImagePropsfunction.The
getImagePropsfunction correctly mergesimgPropsandprops, enhancing code maintainability and readability.
wingkwong
requested review from
wingkwong
and removed request for
jrgarciadev
|
handled in #3946 |
Closes #
Im working on this linear issue: https://linear.app/nextui-inc/issue/ENG-1225/react-doesnt-recognize-disableanimation
📝 Description
[Cause of the error]
The error says we need to remove
disableAnimationprop from<span>element.Currently we are trying to remove this prop from
<img>element instead of<span>. So, I removed those lines of codes.The cause of this error is we did not extract
disabledAnimationprop inuseAvatarfunction's argument object de-structuring so that we accidentally passeddisableAnimationto<span>element withotherProps.[Solution]
So, to avoid passing this prop to
<span>element(which means we shall avoid includingdisableAnimationinotherProps), we added the object de-structuring as a solution for this.⛳️ Current behavior (updates)
the error message in console
🚀 New behavior
Working as fine.
💣 Is this a breaking change (Yes/No):
No
📝 Additional Information
Summary by CodeRabbit