Conversation
Pass content props into getFloatingProps so dismiss and focus handlers are not overridden by consumer spreads. Fixes #806
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesDialog getFloatingProps prop-merge refactor
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
CoverageThis report compares the PR with the base branch. "Δ" shows how the PR affects each metric.
Coverage improved or stayed the same. Great job! Run |
Code reviewLGTM. Matches project patterns for portal Theme refs, Floating UI prop merge, controlled-switch/lazy-mount/RTL tests, or focused bug fixes. No changes requested. |
|
Already implemented on
const consumerStyle = (props as React.HTMLAttributes<HTMLDivElement>).style;
const restProps = { ...props } as Record<string, unknown>;
delete restProps.style;
{...(getFloatingProps as (userProps?: Record<string, unknown>) => Record<string, unknown>)({
...restProps,
style: { outline: 'none', ...styleProp, ...consumerStyle },
...
})}
|
Summary
Fixes #806
Test plan
Summary by CodeRabbit