-
Notifications
You must be signed in to change notification settings - Fork 299
fix(popper): [tooltip,popover] fix popper doms in custom element, cant get zindex #3125
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
WalkthroughThis pull request modifies functions across several modules to improve support for shadow DOM elements. In particular, it adds conditional checks to detect if an element is encapsulated within a shadow DOM by verifying the existence of a Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant isFixed
participant Element
Caller->>isFixed: Call isFixed(el)
isFixed->>Element: Check if 'host' property exists
alt Element has host (shadow DOM)
isFixed->>Element: Set el = el.host
end
isFixed->>Element: Continue fixed positioning check recursively
sequenceDiagram
participant Caller
participant getReferMaxZIndex
participant Reference
Caller->>getReferMaxZIndex: Call getReferMaxZIndex(reference)
getReferMaxZIndex->>Reference: Check if reference is ShadowRoot with a host
alt Reference has host (shadow DOM)
getReferMaxZIndex->>Reference: Set reference = reference.host
end
getReferMaxZIndex->>Reference: Calculate maximum z-index value
sequenceDiagram
participant Caller
participant hasDataTag
participant Element
Caller->>hasDataTag: Call hasDataTag(el, data-tag)
hasDataTag->>Element: Check for host property (shadow DOM)
alt Element has host
hasDataTag->>Element: Set el = el.host
end
hasDataTag->>Element: Verify matching data attribute
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🪧 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
CodeRabbit Configuration File (
|
Walkthrough此PR修复了在自定义元素内部使用popper时,无法正确获取父节点z-index的问题。通过处理shadowRoot的情况,确保popper在自定义元素中能够正常工作。 Changes
|
@@ -180,11 +180,15 @@ export const colToVisible = ($table, column, move) => { | |||
} | |||
|
|||
export const hasDataTag = (el, value) => { | |||
// el可能为shadow-root,shadow-root没有getAttribute方法 | |||
if (!el || !value || !el.getAttribute) { | |||
if (!el || !value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The removal of the check for el.getAttribute
could lead to runtime errors if el
is not a valid DOM element or does not have the getAttribute
method. Consider adding a check to ensure el
is a valid DOM element before calling getAttribute
. This will prevent potential runtime errors.
WalkthroughThis PR fixes the issue where the parent node z-index cannot be properly obtained when using popper inside a custom element. By handling shadowRoot situations, make sure that popper works properly in custom elements. Changes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/vue-hooks/package.json (2)
5-5
: New "author" Field Added
An empty "author" field is now included. Consider filling this with the appropriate author information (e.g., name and/or email) to improve package metadata clarity.
7-7
: "Keywords" Field Introduced
A "keywords" field (currently empty) has been added. To enhance package discoverability, consider adding relevant keywords in future updates.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/renderless/package.json
(2 hunks)packages/utils/package.json
(2 hunks)packages/vue-hooks/package.json
(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/utils/package.json
- packages/renderless/package.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (3)
packages/vue-hooks/package.json (3)
3-3
: Version Bump Confirmation
The version update from "3.21.0" to "3.21.1" is correctly reflected and indicates a minor release.
6-6
: Explicit License Declaration
Adding the "license" field with the value "ISC" improves clarity regarding the terms under which the package is distributed.
17-19
: Workspace Dependency Version Specifier Verification
The dependency declaration for "@opentiny/vue-common" uses the "workspace:~" specifier. Please verify that this specifier works as intended in the monorepo build environment and aligns with the versioning policies across packages.
0be30ad
to
83b710b
Compare
PR
PR Checklist
修复popper用在自定义元素内部时,向上找父节点报错的问题。 ---- 同步代码
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit