fix(dialog-box): dialog pop-up mask layer animation is not working (#… #3637
Conversation
WalkthroughThe changes add conditional modal overlay styling and shared fade keyframe animations in the dialog LESS, and update the Vue dialog wrapper to include a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DialogComponent
participant Stylesheet
User->>DialogComponent: request open dialog (modal = true/false)
DialogComponent->>DialogComponent: render wrapper with base classes
DialogComponent->>Stylesheet: include `has-modal` class if modal = true
Stylesheet->>DialogComponent: apply overlay background + modal-fade animations when `has-modal` present
Stylesheet->>DialogComponent: apply standard animations when `has-modal` absent
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
修复，去除 v-modal 背景使用 tiny-dialog-box__wrapper 背景作为遮罩 
|
|
可能的问题
|
Possible problems
|
|
Can somebody review ? |
|
需要验证下 Modal 组件的遮罩是否受影响 |
It is necessary to verify whether the mask of the Modal component is affected. |
| @@ -270,15 +275,43 @@ | |||
| } | |||
| } | |||
|
|
|||
| .tiny-dialog-box__wrapper.has-modal { | |||
| background-color: rgba(0, 0, 0, 0.2); | |||
There was a problem hiding this comment.
按照规范，此处将设置成：background: var(--tv-DialogBox-mask-bg-color);
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
packages/theme/src/dialog-box/index.less (1)
278-281: Good: mask color uses design tokenUsing var(--tv-DialogBox-mask-bg-color) aligns with theming and prior feedback.
🧹 Nitpick comments (2)
packages/theme/src/dialog-box/index.less (1)
369-381: Optional: also fade opacity during enlarge-out for smoother exitScaling to 0 without fading can feel abrupt if content remains crisp until the last frame. Fading opacity alongside scale is a subtle enhancement.
@keyframes enlarge-out { 0% { - transform: scale(1); + transform: scale(1); + opacity: 1; } 50% { - transform: scale(0.5); + transform: scale(0.5); + opacity: 0.5; } 100% { - transform: scale(0); + transform: scale(0); + opacity: 0; } }packages/vue/src/dialog-box/src/pc.vue (1)
21-21: Prefer object-style conditional class instead of empty string placeholderCleaner and avoids emitting an empty class when modal is false.
- :class="['tiny-dialog-box__wrapper', dialogClass, modal ? 'has-modal' : '']" + :class="['tiny-dialog-box__wrapper', dialogClass, { 'has-modal': modal }]"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/theme/src/dialog-box/index.less(4 hunks)packages/vue/src/dialog-box/src/pc.vue(1 hunks)
🔇 Additional comments (2)
packages/theme/src/dialog-box/index.less (2)
299-307: LGTM: shared modal fade keyframes addedmodal-fade-in/out keyframes are straightforward and reusable for all mask animations.
Also applies to: 308-315
201-208: Confirm: .v-modal is behind the dialog wrapper — no change requiredShort reason: PopupManager.openModal() is given PopupManager.nextZIndex() for the modal DOM, and the dialog wrapper is assigned dom.style.zIndex = PopupManager.nextZIndex() immediately afterwards — so the wrapper always receives a higher z-index than .v-modal.
Files to note:
- packages/utils/src/popup-manager/index.ts — openModal sets modalDom.style.zIndex = zIndex.toString() and manages the modal stack.
- packages/vue-hooks/src/vue-popup.ts — openFn calls PopupManager.openModal(..., PopupManager.nextZIndex(), ...) then sets dom.style.zIndex = PopupManager.nextZIndex().toString().
- packages/theme/src/dialog-box/index.less and packages/theme-saas/src/dialog-box/index.less — .v-modal CSS (saas variant adds bg/opacity); visual transparency does not affect the runtime z-index ordering enforced by the JS above.
Action: ignore the original concern — runtime code ensures .v-modal will not float above the wrapper mask in normal flows.
Likely an incorrect or invalid review comment.
| &.tiny-dialog-box__wrapper { | ||
| animation: modal-fade-in 0.5s; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure mask fade has correct fill-mode to avoid end-frame flicker
Add fill-mode to the wrapper’s modal fade animations. Without it, the mask can momentarily snap back to full opacity at the end of leave or have a first-frame pop on enter.
.dialog-slideRight-enter-active {
animation: slideRight 0.5s ease-in forwards;
&.tiny-dialog-box__wrapper {
- animation: modal-fade-in 0.5s;
+ animation: modal-fade-in 0.5s both;
}
}
.dialog-slideRight-leave-active {
animation: slideRightout 0.5s ease-in forwards;
&.tiny-dialog-box__wrapper {
- animation: modal-fade-out 0.5s;
+ animation: modal-fade-out 0.5s forwards;
}
}Also applies to: 239-242
🤖 Prompt for AI Agents
In packages/theme/src/dialog-box/index.less around lines 232-235 (and also apply
same change at 239-242), the modal-fade-in/out animations lack an
animation-fill-mode which causes end-frame flicker; add an appropriate
animation-fill-mode (preferably "both" to preserve start and end states) to the
wrapper’s animation declarations so the mask retains its intended opacity at
both enter and leave transitions.
| &.tiny-dialog-box__wrapper { | ||
| animation: modal-fade-in 0.3s; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Unify fade enter/leave fill-mode for consistency
Apply the same fill-mode pattern to dialog-fade enter/leave to prevent visual popping at the start/end of the animation.
.dialog-fade-enter-active {
animation: dialog-fade-in 0.3s;
&.tiny-dialog-box__wrapper {
- animation: modal-fade-in 0.3s;
+ animation: modal-fade-in 0.3s both;
}
}
.dialog-fade-leave-active {
.@{dialog-box-prefix-cls} {
animation: dialog-fade-out 3s;
}
animation: dialog-fade-out 0.3s;
&.tiny-dialog-box__wrapper {
- animation: modal-fade-out 0.3s;
+ animation: modal-fade-out 0.3s forwards;
}
}Also applies to: 294-297
🤖 Prompt for AI Agents
In packages/theme/src/dialog-box/index.less around lines 284-287 (and similarly
lines 294-297), the fade animations lack a fill-mode which causes visual popping
at animation start/end; update the relevant selectors (the
.tiny-dialog-box__wrapper and the dialog-fade enter/leave selectors referenced
at 294-297) to include animation-fill-mode: both (or add animation: ... 0.3s
both) so the animated element retains start/end styles during the animation to
prevent popping.
| .dialog-fade-leave-active { | ||
| .@{dialog-box-prefix-cls} { | ||
| animation: dialog-fade-out 3s; | ||
| } | ||
| animation: dialog-fade-out 0.3s; | ||
| &.tiny-dialog-box__wrapper { | ||
| animation: modal-fade-out 0.3s; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
3s fade-out on inner dialog likely unintended; align to 0.3s
A 3s fade on the inner panel during leave is disproportionately long vs. the 0.3s wrapper fade and other transitions. This looks like a typo.
.dialog-fade-leave-active {
.@{dialog-box-prefix-cls} {
- animation: dialog-fade-out 3s;
+ animation: dialog-fade-out 0.3s;
}
animation: dialog-fade-out 0.3s;
&.tiny-dialog-box__wrapper {
animation: modal-fade-out 0.3s forwards;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .dialog-fade-leave-active { | |
| .@{dialog-box-prefix-cls} { | |
| animation: dialog-fade-out 3s; | |
| } | |
| animation: dialog-fade-out 0.3s; | |
| &.tiny-dialog-box__wrapper { | |
| animation: modal-fade-out 0.3s; | |
| } | |
| } | |
| .dialog-fade-leave-active { | |
| .@{dialog-box-prefix-cls} { | |
| animation: dialog-fade-out 0.3s; | |
| } | |
| animation: dialog-fade-out 0.3s; | |
| &.tiny-dialog-box__wrapper { | |
| animation: modal-fade-out 0.3s forwards; | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/theme/src/dialog-box/index.less around lines 289 to 297, the inner
dialog rule sets animation: dialog-fade-out 3s which is likely a typo and
inconsistent with the wrapper's 0.3s duration; change the inner
.@{dialog-box-prefix-cls} animation duration from 3s to 0.3s so both inner panel
and wrapper use the same 0.3s fade-out timing and maintain consistent
transitions.
| .enlarge-enter-active { | ||
| animation: enlarge-in 350ms linear; | ||
| animation: enlarge-in 0.35s linear; | ||
| &.tiny-dialog-box__wrapper { | ||
| animation: modal-fade-in 0.35s; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply fill-mode to enlarge enter/leave wrapper fades
Keep consistency with other transitions and avoid end-of-animation flashes.
.enlarge-enter-active {
animation: enlarge-in 0.35s linear;
&.tiny-dialog-box__wrapper {
- animation: modal-fade-in 0.35s;
+ animation: modal-fade-in 0.35s both;
}
}
.enlarge-leave-active {
animation: enlarge-out 0.35s linear;
&.tiny-dialog-box__wrapper {
- animation: modal-fade-out 0.35s;
+ animation: modal-fade-out 0.35s forwards;
}
}Also applies to: 348-353
🤖 Prompt for AI Agents
In packages/theme/src/dialog-box/index.less around lines 341-346 (and also for
the corresponding leave rules at 348-353), the enter/leave active animation
rules lack an animation-fill-mode which causes end-of-animation flashes; add
animation-fill-mode: both to the .enlarge-enter-active and its nested
.tiny-dialog-box__wrapper rule, and likewise add animation-fill-mode: both to
the .enlarge-leave-active and its nested wrapper rule so the animated state is
preserved before and after the animation.
| @@ -18,7 +18,7 @@ | |||
| > | |||
| <div | |||
| v-show="visible" | |||
| :class="['tiny-dialog-box__wrapper', dialogClass]" | |||
| :class="['tiny-dialog-box__wrapper', dialogClass, modal ? 'has-modal' : '']" | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Double-check modalAppendToBody interaction and pointer event flow
With the mask now on the wrapper, ensure:
- When modalAppendToBody is true, the wrapper actually overlays the viewport (not trapped by transformed ancestors).
- .v-modal (transparent) doesn’t sit above the wrapper and eat clicks.
- Mobile viewport/scroll-lock behavior remains correct.
If issues surface, consider gating has-modal with actual append-to-body state or ensuring wrapper is teleported to body when masking.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate appendToBody/modalAppendToBody handling for dialog-box"
rg -n --no-heading -A 6 -B 6 -S 'appendToBody|modalAppendToBody' packages/vue/src/dialog-box
echo "Check any teleport/portal usage in dialog-box code"
rg -n --no-heading -A 2 -B 2 -S 'Teleport|teleport|appendChild' packages/vue/src/dialog-box packages/vue-renderlessLength of output: 3989
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "==== pc.vue (showing with line numbers) ===="
nl -ba packages/vue/src/dialog-box/src/pc.vue | sed -n '1,360p' || true
echo
echo "==== mobile-first.vue (showing with line numbers) ===="
nl -ba packages/vue/src/dialog-box/src/mobile-first.vue | sed -n '1,360p' || true
echo
echo "==== Search for Teleport / teleport in packages/vue ===="
rg -n --no-heading -S 'Teleport|teleport' packages/vue || true
echo
echo "==== Search for appendToBody/modalAppendToBody usage across repo ===="
rg -n --no-heading -S 'appendToBody|modalAppendToBody' || true
echo
echo "==== Search for class 'has-modal' usage across repo ===="
rg -n --no-heading -S "has-modal" || trueLength of output: 8541
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "==== packages/vue/src/dialog-box/src/pc.vue ===="
sed -n '1,400p' packages/vue/src/dialog-box/src/pc.vue || true
echo
echo "==== packages/vue/src/dialog-box/src/mobile-first.vue ===="
sed -n '1,400p' packages/vue/src/dialog-box/src/mobile-first.vue || true
echo
echo "==== packages/vue/src/dialog-box/src/index.ts (props) ===="
sed -n '1,240p' packages/vue/src/dialog-box/src/index.ts || true
echo
echo "==== packages/renderless/src/dialog-box/index.ts ===="
sed -n '1,420p' packages/renderless/src/dialog-box/index.ts || true
echo
echo "==== packages/vue-hooks/src/vue-popup.ts ===="
sed -n '1,420p' packages/vue-hooks/src/vue-popup.ts || true
echo
echo "==== packages/theme/src/dialog-box/index.less (around has-modal) ===="
sed -n '250,320p' packages/theme/src/dialog-box/index.less || trueLength of output: 30798
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Search for tiny-dialog-box__wrapper usages in theme and styles"
rg -n --no-heading -S "tiny-dialog-box__wrapper" || true
echo
echo "Show full dialog-box theme file"
sed -n '1,400p' packages/theme/src/dialog-box/index.less || true
echo
echo "Search for PopupManager implementation in repo"
rg -n --no-heading -S "class PopupManager|export const PopupManager|function PopupManager|PopupManager =" || true
echo
echo "Search for openModal implementation"
rg -n --no-heading -S "openModal(" || true
echo
echo "Search for v-modal or modal class used by PopupManager (common names)"
rg -n --no-heading -S "v-modal|modal-mask|pop-modal|popLockClass|pop-modal" || trueLength of output: 12889
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "==== packages/utils/src/popup-manager/index.ts ===="
sed -n '1,400p' packages/utils/src/popup-manager/index.ts || trueLength of output: 6949
Action required — modalAppendToBody vs wrapper mask can be out-of-sync
Short summary: I verified the code paths. modalAppendToBody controls where PopupManager inserts the global .v-modal, while appendToBody controls whether the dialog wrapper (tiny-dialog-box__wrapper) is moved to body. The wrapper now carries the visual mask (.has-modal). That combination (modalAppendToBody === true && appendToBody === false — both are defaults) can leave the wrapper trapped by transformed ancestors (so it won't cover the viewport) or otherwise cause layering/pointer issues.
What I checked (evidence):
- packages/vue/src/dialog-box/src/pc.vue — wrapper class binding (current line):
:class="['tiny-dialog-box__wrapper', dialogClass, modal ? 'has-modal' : '']" - packages/renderless/src/dialog-box/index.ts — appends parent.$el to document.body only when props.appendToBody is true.
- packages/vue-hooks/src/vue-popup.ts — passes props.modalAppendToBody to PopupManager.openModal.
- packages/utils/src/popup-manager/index.ts — openModal appends the modal DOM to document.body or dom.parentNode and manages z-index.
- packages/theme/src/dialog-box/index.less — .tiny-dialog-box__wrapper.has-modal provides the mask; .v-modal also exists.
Recommended fixes (pick one):
- Minimal / safest: only apply has-modal when the wrapper actually overlays the viewport:
- packages/vue/src/dialog-box/src/pc.vue — change the class binding to gate has-modal:
- :class="['tiny-dialog-box__wrapper', dialogClass, modal ? 'has-modal' : '']"
+ :class="['tiny-dialog-box__wrapper', dialogClass, (modal && (appendToBody || !modalAppendToBody)) ? 'has-modal' : '']"Rationale: if modalAppendToBody === true but the wrapper is not appended to body, don't apply the wrapper mask (avoid trapped/incomplete mask). That leaves PopupManager/.v-modal to provide the mask.
- More robust (recommended if you want the visual mask always placed on the wrapper): ensure the wrapper is teleported to body whenever modal masking is used:
- packages/renderless/src/dialog-box/index.ts — in watchVisible/mounted/unMounted, treat modalAppendToBody the same as appendToBody for moving the wrapper:
- if (props.appendToBody) {
+ if (props.appendToBody || (props.modal && props.modalAppendToBody)) {
document.body.appendChild(el)
}(and mirror the same predicate when removing the element in unMounted).
Rationale: keeps visual mask (has-modal) on the wrapper and avoids transformed-ancestor containment.
Extra checks to run after applying either fix:
- Verify stacking/z-index order between .v-modal and the wrapper still yields the correct visual and pointer behavior.
- Test cases where the component is inside a transformed ancestor.
- Mobile scroll-lock and touch behavior (PopupManager adds touch handlers and popLockClass) — ensure no regressions.
Files to review:
- packages/vue/src/dialog-box/src/pc.vue (class binding)
- packages/renderless/src/dialog-box/index.ts (append/remove logic)
- packages/vue-hooks/src/vue-popup.ts (modalAppendToBody usage)
- packages/utils/src/popup-manager/index.ts (openModal/z-index behavior)
- packages/theme/src/dialog-box/index.less (mask styles)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| :class="['tiny-dialog-box__wrapper', dialogClass, modal ? 'has-modal' : '']" | |
| :class="['tiny-dialog-box__wrapper', dialogClass, (modal && (appendToBody || !modalAppendToBody)) ? 'has-modal' : '']" |
🤖 Prompt for AI Agents
packages/vue/src/dialog-box/src/pc.vue lines 21-21: the wrapper class binding
applies .has-modal whenever modal is true, but modalAppendToBody can cause the
global .v-modal to be appended to body while the wrapper remains inside its
original ancestor, causing mask/coverage and pointer issues; fix by gating the
has-modal class so it is only added when the wrapper is actually appended to
body (i.e., modal && (appendToBody || modalAppendToBodyEquivalentFlag)), or
alternatively update packages/renderless/src/dialog-box/index.ts to treat
modalAppendToBody the same as appendToBody and move the wrapper to document.body
whenever modal masking is used (and mirror removal in unMounted) — pick one:
either change the class condition in pc.vue to only add has-modal when the
wrapper is moved to body, or change the renderless logic to ensure the wrapper
is teleported to body when modal masking is enabled.
PR
PR Checklist
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: #3636
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Possible problems
Summary by CodeRabbit
New Features
Style