fix(dropdown): correct initial animation direction to match fallback placement #4288
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: bf3790c The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Rate limit exceeded@ryo-manba has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 33 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe pull request introduces several changes to the dropdown component. A new dependency, Changes
Assessment against linked issues
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: 2
🧹 Outside diff range and nitpick comments (1)
packages/components/dropdown/src/use-dropdown.ts (1)
167-167: Simplify placement assignment in getPopoverPropsIn
getPopoverProps, the placement is assigned usingplacement: placement || DEFAULT_PLACEMENT. Sinceplacementalready defaults toDEFAULT_PLACEMENT, this may be redundant. Consider simplifying the assignment.Apply this diff to streamline the code:
return { state, - placement: placement || DEFAULT_PLACEMENT, + placement, ref: popoverRef, disableAnimation, // ... }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
packages/components/dropdown/package.json(1 hunks)packages/components/dropdown/src/use-dropdown.ts(5 hunks)packages/components/dropdown/stories/dropdown.stories.tsx(2 hunks)
🔇 Additional comments (3)
packages/components/dropdown/src/use-dropdown.ts (1)
142-151: Ensure correct handling of overlay positioning props
The useOverlayPosition hook utilizes the new positioning props (offset, crossOffset, shouldFlip, containerPadding). Verify that these props are correctly passed down and that their default values align with the expected behavior.
packages/components/dropdown/package.json (1)
51-51: Verify the new dependency version and check for security advisories
The dependency @react-aria/overlays version "3.24.0" has been added. Ensure that this version is compatible with other dependencies and is free from known vulnerabilities.
Run the following script to check for the latest version and any security advisories:
✅ Verification successful
Dependency version is up-to-date and secure
The version 3.24.0 of @react-aria/overlays is currently the latest version available on npm, and there are no known security vulnerabilities reported for this package.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the latest version and security advisories for @react-aria/overlays
# Get the latest version from npm
npm view @react-aria/overlays version
# Check for known vulnerabilities using GitHub's security advisories
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "@react-aria/overlays") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
Length of output: 400
packages/components/dropdown/stories/dropdown.stories.tsx (1)
143-143: Verify default value impact for the new 'label' prop
The label prop has been added to the Template function with a default value of "Trigger". Ensure that this change does not adversely affect existing stories that may rely on the previous hardcoded label.
ryo-manba
changed the title
Closes #4251
📝 Description
When the fallback placement is used, the animation did not follow the final placement.
As a result, the popup position and its animation did not match.
⛳️ Current behavior (updates)
The popup shows in the correct place, but the initial animation starts from the wrong direction, causing a flicker.
dropdown-animation-before.mov
🚀 New behavior
The animation now correctly aligns with the final placement.
Changed the code to use
useOverlayPositionsimilar to the Tooltip implementation.dropdown-animation-after.mov
💣 Is this a breaking change (Yes/No):
No.
📝 Additional Information
Dropdown docs do not mention
placement, but that will be addressed separately.Summary by CodeRabbit
New Features
containerPadding,offset,crossOffset, andshouldFlip.Bug Fixes
Documentation