fix(popover): fix the clickoutside bug in the pop-up component

[shenjunjian]

PR

点击 popover 组件外面的webcompont 不能关闭的bug

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our Commit Message Guidelines
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Documentation content changes
• Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

• Yes
• No

Other information

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Refined popover event handling to correctly manage interactions with nested custom elements, improving reliability when used with web components.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The popover component's shadow DOM target normalization logic in handleDocumentClick is refined to only replace the target when the shadowRoot contains the component root and popperElm exists, preventing unnecessary target replacement for sibling custom elements.

Changes

Cohort / File(s)	Summary
Popover Shadow DOM Logic packages/renderless/src/popover/index.ts	Tightened target normalization in handleDocumentClick to distinguish between containment within custom components versus sibling custom elements by adding additional conditions for shadowRoot handling

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Verify the logic correctness: ensure the new condition (shadowRoot.contains($el) && popperElm) captures the intended use case
• Test edge cases: confirm behavior with nested custom elements and various shadow DOM configurations
• Validate that sibling custom elements no longer trigger unintended target replacements

Poem

🐰 A shadow DOM bug was tamed with care,
Target checks now discern what's there,
Inside or sibling, we now tell apart,
Logic refined—a developer's art!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(popover): fix the clickoutside bug in the pop-up component' directly addresses the main change—fixing a click-outside bug in the popover component when clicking on web components, which is clearly supported by the file summary.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch shen/fix-popover-close

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

packages/renderless/src/popover/index.ts (1)

186-205: Shadow DOM containment check correctly narrows target rewriting; consider a small robustness tweak and verify same-host behavior

The new guard:

if (target?.shadowRoot && target.shadowRoot.contains($el) && popperElm) {
  target = state.webCompEventTarget as HTMLElement
}

does what the PR intends: it only rewrites target when the click comes from a web component host whose shadowRoot actually contains the popover root, so sibling custom elements no longer reuse state.webCompEventTarget and block “click‑outside” closing. This is a good narrowing of the previous behavior.

Two follow‑ups to consider:

1. Guard the cached target for safety

To avoid ever assigning an undefined/non‑HTMLElement into target and to make the intent clearer, you could add an extra check:

-    if (target?.shadowRoot && target.shadowRoot.contains($el) && popperElm) {
-      target = state.webCompEventTarget as HTMLElement
-    }
+    if (
+      target?.shadowRoot &&
+      target.shadowRoot.contains($el) &&
+      popperElm &&
+      state.webCompEventTarget instanceof HTMLElement
+    ) {
+      target = state.webCompEventTarget
+    }

This keeps behavior the same in the normal cases but is more defensive.

2. Verify clicks inside the same web component but outside popover/reference

Because state.webCompEventTarget is only updated in handleClick (bound to referenceElm), later clicks inside the same web component but outside the popover/reference may still reuse the last cached internal target when target.shadowRoot.contains($el) is true. Depending on desired UX, that might mean such clicks are treated as “inside” and won’t close the popover.

Please explicitly verify this flow:

• Popover inside a web component host.
• Open popover via referenceElm.
• Click another area inside the same web component but outside both $el and popperElm.

Confirm whether you expect the popover to close or stay open in that case and adjust logic if needed (for example by clearing or updating state.webCompEventTarget more narrowly).

3. Unreachable code at the end of handleDocumentClick

The final state.showPopper = false after the if/else with return is unreachable and can be safely removed:

-    } else {
-      state.showPopper = false
-      return true
-    }
-
-    state.showPopper = false
+    } else {
+      state.showPopper = false
+      return true
+    }

Overall, the main fix looks solid; the above are just small robustness/cleanup suggestions and a behavior check for a subtle edge case inside the same host.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5bdfec1 and 143cbb9.

📒 Files selected for processing (1)

• packages/renderless/src/popover/index.ts (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: PR E2E Test (pnpm test:e2e3)