feat(Popper): add dir prop for RTL/LTR support#2610
Conversation
📝 WalkthroughWalkthroughAdds optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/core/src/Popper/PopperContent.vue (1)
177-182: Newdirprop wiring looks correct.
dir?: Directionis optional and correctly typed against the sharedDirectionalias. Pairing withuseDirection(computed(() => props.dir))at Line 233 properly falls back toConfigProvider’s direction and finally'ltr', preserving existing behavior for unset props.One small note: the phrasing “when applicable” in the JSDoc is a bit vague — consider clarifying that the direction only affects the computed
transform-origin(specifically thestart/endalignment on top/bottom placements when the arrow is hidden), and does not automatically flipside. Users in RTL layouts who wantside: 'right'to visually appear on the logical start still need to mirrorsidethemselves.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/Popper/PopperContent.vue` around lines 177 - 182, Update the JSDoc for the dir prop in PopperContent.vue to explicitly state that dir?: Direction (wired to useDirection(computed(() => props.dir))) only affects computed transform-origin (i.e., start/end alignment on top/bottom placements when the arrow is hidden) and does not flip or remap the side prop — users must mirror side themselves in RTL; make the wording concise and replace "when applicable" with this specific behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/core/src/Popper/PopperContent.vue`:
- Around line 177-182: Update the JSDoc for the dir prop in PopperContent.vue to
explicitly state that dir?: Direction (wired to useDirection(computed(() =>
props.dir))) only affects computed transform-origin (i.e., start/end alignment
on top/bottom placements when the arrow is hidden) and does not flip or remap
the side prop — users must mirror side themselves in RTL; make the wording
concise and replace "when applicable" with this specific behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6de00e3e-048d-4dbb-96dd-7ebbccc66361
📒 Files selected for processing (2)
packages/core/src/Popper/PopperContent.vuepackages/core/src/Popper/utils.ts
🔗 Linked issue
❓ Type of change
📚 Description
📸 Screenshots (if appropriate)
📝 Checklist