Skip to content

Conversation

@zzcr
Copy link
Member

@zzcr zzcr commented Oct 28, 2025

fix(drawer): 修复drawer组件中表格组件全屏无法正常展示问题

PR

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

  • Style
    • Improved drawer rendering behavior during visibility state transitions on both mobile and desktop layouts

@github-actions github-actions bot added the bug Something isn't working label Oct 28, 2025
@coderabbitai
Copy link

coderabbitai bot commented Oct 28, 2025

Walkthrough

Two drawer component templates receive identical style binding updates. A conditional transform property is added to each component's main container style object, setting the value to 'none' when visible and null when hidden, without affecting existing logic or control flow.

Changes

Cohort / File(s) Summary
Conditional Transform Styling in Drawer Components
packages/vue/src/drawer/src/mobile-first.vue, packages/vue/src/drawer/src/pc.vue
Added conditional inline transform style property to main drawer container, setting to 'none' when visible and null when hidden

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Both changes are identical in nature and scope, making them straightforward to verify
  • Verify the conditional logic matches visibility state in both components
  • Confirm consistency of the style binding syntax across both files

Poem

🐰 A transform so fine, when drawers appear,
Setting none with pride, crystal clear,
Mobile and desktop, both now aligned,
Smooth style bindings, perfectly designed! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "fix(drawer): Fixed the problem that the table component in the drawer component cannot be displayed normally in full screen" is fully related to the main change in the changeset. The PR objectives explicitly state that the goal is to fix an issue where a table component inside the drawer cannot display correctly in full-screen mode, which matches the title's description. The code changes add conditional transform styles to the drawer components in both mobile-first.vue and pc.vue to address this display issue. The title is clear and specific enough that a teammate reviewing the commit history would understand this fixes a full-screen display problem with table components in drawers. While the title is somewhat verbose (19 words), it avoids vague terms and provides concrete context about what is being fixed.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch zzc/fix-drawer-position-fixed-bug-1028

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

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6f28219 and bd2cf6d.

📒 Files selected for processing (2)
  • packages/vue/src/drawer/src/mobile-first.vue (1 hunks)
  • packages/vue/src/drawer/src/pc.vue (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)
🔇 Additional comments (1)
packages/vue/src/drawer/src/mobile-first.vue (1)

35-36: Verify that drawer animations work correctly with the new inline transform style.

The inline transform: 'none' style (line 35) will override the Tailwind transform classes when the drawer is visible. Existing tests only verify final CSS properties and dimensions—they don't test animation smoothness or transition timing. The placement test checks that CSS properties are set (left: 0px, etc.), and the dragable test checks that dimensions change, but neither verifies that the drawer slides smoothly when opening or closing, or that the transition animation works correctly with the inline style override.

Manually verify:

  1. Drawer slides smoothly when opened/closed in all placements (top/bottom/left/right)
  2. Animation timing remains consistent with the duration-200 class
  3. Drag functionality works without animation glitches when dragable is enabled

Comment on lines +35 to +36
zIndex,
transform: state.visible ? 'none' : null
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify Vue transition compatibility with the inline transform style.

The transform: 'none' inline style will override any CSS transforms defined in the Vue transition classes (e.g., .drawer-slide-left-enter-active). This is necessary to remove the containing block and fix the fullscreen table issue, but it may interfere with Vue's transition system.

Please verify that:

  1. The Vue transition classes for drawer-slide-${placement} still work correctly with the inline transform
  2. Enter and leave transitions animate smoothly without jumping
  3. The transition doesn't conflict during the enter/leave phases when Vue is adding/removing classes

Additionally, consider whether the transition CSS should be updated to avoid using transform if the inline style will always override it when visible.


🏁 Script executed:

#!/bin/bash
# Search for the drawer transition class definitions
rg -n --type=less --type=css --type=scss -A5 'drawer-slide|drawer-fade' 

# Check if there are styles that use transform for drawer animations
ast-grep --pattern $'drawer {
  $$$
  transform: $_;
  $$$
}'

Length of output: 28



🏁 Script executed:

#!/bin/bash
# Search for drawer transition CSS without strict file type
rg -n 'drawer-slide|drawer-fade' --type=css --type=less -A5

# If above finds nothing, search all files
if [ $? -ne 0 ]; then
  echo "=== Searching in all files for drawer-slide ==="
  rg -n 'drawer-slide|drawer-fade' -A5 --max-count=20
fi

# Look for Vue component style blocks or animation/keyframe definitions
echo "=== Searching for animation/keyframe definitions ==="
rg -n 'animation:|keyframes|@keyframes' --type=css --type=less | grep -i drawer

# Check if there are dedicated style files in drawer directory
echo "=== Files in drawer directory ==="
fd -e css -e less -e scss -e vue packages/vue/src/drawer 2>/dev/null | head -20

Length of output: 11155



🏁 Script executed:

#!/bin/bash
# Get complete keyframe definitions to see what properties they animate
echo "=== Complete slideLeft keyframe ==="
rg -n 'slideLeft' -A15 packages/theme/src/drawer/index.less | head -40

echo "=== Complete slideTop keyframe ==="
rg -n '@keyframes slideTop' -A10 packages/theme/src/drawer/index.less

echo "=== Complete slideRight keyframe ==="
rg -n '@keyframes slideRight' -A10 packages/theme/src/drawer/index.less

Length of output: 1581


Critical Animation Breaking Issue

The inline transform: 'none' when state.visible is true will break the drawer enter animations. All slide keyframes (slideLeft, slideRight, slideTop, slideBottom) use transform: translateX() or transform: translateY() to animate position. When the drawer opens, the inline style will override these animation values, preventing the drawer from sliding in.

The drawer will appear instantly without animation on enter, and animations will only work on leave (when transform is null).

Required: Replace the inline transform style with a CSS class approach or use CSS animation-fill-mode to preserve animation behavior while maintaining the containing block fix.

🤖 Prompt for AI Agents
In packages/vue/src/drawer/src/pc.vue around lines 35-36, the inline style
setting transform: state.visible ? 'none' : null breaks enter animations by
overriding keyframed translate transforms; remove the inline transform and
instead toggle a CSS class (e.g., .contain-fix) when visible that applies the
containing/z-index fix without setting transform, or alternatively ensure the
animation uses animation-fill-mode (and remove the inline transform) so keyframe
translateX/translateY can run on enter; update the template to add/remove the
class and add the corresponding CSS rule to achieve the containing behavior
without an inline transform override.

@kagol kagol merged commit 839c8bc into dev Oct 28, 2025
10 of 11 checks passed
@kagol kagol deleted the zzc/fix-drawer-position-fixed-bug-1028 branch October 28, 2025 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants