fix(Combobox): keep content open when interacting with associated label#2686
Conversation
A `<label>` tied (via `for`) to a control inside the combobox forwards its click/focus to that control. Without accounting for this, clicking such a label while open dismissed the content on `pointerdown`, then the forwarded click/focus immediately re-opened it.
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds label-aware dismiss prevention to Combobox: a helper treats labels whose ChangesCombobox Label-aware Dismiss Prevention
Possibly Related PRs
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 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: |
The 'stays open' assertion ran after a single nextTick, but a real dismiss is emitted after an internal nextTick too — so the test could pass even if the fix failed to prevent dismiss. Match the wait used in the dismiss test to actually guard against that regression. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🔗 Linked issue
❓ Type of change
📚 Description
A
<label>associated (viafor) with a control inside the combobox forwards its click/focus to that control. Without accounting for this, clicking such a label while the combobox is open dismissed the content onpointerdown, and the forwarded click/focus then immediately re-opened it, which felt broken.Added an
isEventTargetWithinComboboxhelper that treats a label whosecontrollives inside the combobox as "inside", sopointerDownOutside/focusOutsideno longer dismiss in that case.Added two tests: an associated label keeps the content open, an unrelated label still dismisses.
📝 Checklist
Summary by CodeRabbit
Bug Fixes
Tests