-
Notifications
You must be signed in to change notification settings - Fork 7.4k
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
fix: keepAlive not working for popup appendToMain
#5666
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request refines the element targeting logic in the drawer and modal components by updating their computed Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DrawerComponent
participant ModalComponent
participant DOM
participant Router
User->>DrawerComponent: Opens Drawer
DrawerComponent->>DOM: Evaluate getAppendTo (using selector "#main-content > div:not(.absolute)>div")
Note over DrawerComponent: Determines target element for appending
User->>ModalComponent: Opens Modal
ModalComponent->>DOM: Evaluate getAppendTo & check isDeactivated
Note over ModalComponent: Applies conditional "hidden" class if needed
Router->>ModalComponent: Loads component with keepAlive state
Router->>DrawerComponent: Loads component with keepAlive state
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
🪧 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 (
|
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/@core/ui-kit/popup-ui/src/drawer/drawer.vue
(1 hunks)packages/@core/ui-kit/popup-ui/src/modal/modal.vue
(2 hunks)playground/src/router/routes/modules/examples.ts
(2 hunks)playground/src/views/examples/drawer/in-content-demo.vue
(2 hunks)playground/src/views/examples/drawer/index.vue
(1 hunks)playground/src/views/examples/modal/in-content-demo.vue
(3 hunks)playground/src/views/examples/modal/index.vue
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Check (windows-latest)
- GitHub Check: Check (ubuntu-latest)
- GitHub Check: Test (windows-latest)
- GitHub Check: Lint (windows-latest)
- GitHub Check: post-update (windows-latest)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: post-update (ubuntu-latest)
🔇 Additional comments (12)
playground/src/views/examples/modal/index.vue (1)
17-18
: Good addition of component name definitionAdding the component name with
defineOptions
is a good practice. This is essential for proper functioning of the keep-alive caching mechanism in Vue, which addresses the PR objective of fixing keep-alive functionality for popups.playground/src/views/examples/drawer/index.vue (1)
16-16
: Good addition of component name definitionAdding the component name is necessary for proper keep-alive functionality. This change works with the route metadata update mentioned in the summary (adding
keepAlive: true
) to ensure the drawer component state is preserved when navigating away.playground/src/views/examples/drawer/in-content-demo.vue (1)
2-3
: Good test implementation for keep-alive functionalityThe addition of the reactive
value
variable and Input component creates a good test case for verifying keep-alive functionality. When keep-alive is working correctly, the input value should be preserved when navigating away and back to this component.Also applies to: 6-9, 24-24
packages/@core/ui-kit/popup-ui/src/drawer/drawer.vue (1)
130-132
:✅ Verification successful
Fixed keep-alive functionality with more specific selector
This change directly addresses the PR objective by refining the target element selector when
appendToMain
is true. Using a more specific CSS selector (>div:not(.absolute)>div
) ensures the drawer will be appended to the correct DOM element, which is essential for proper component caching with keep-alive.
🏁 Script executed:
#!/bin/bash # Check if there's a similar change in the modal component for consistency echo "Checking for similar implementation in modal component:" rg -A3 -B1 "getAppendTo = computed" packages/@core/ui-kit/popup-ui/src/modal/Length of output: 598
Consistent keep-alive fix confirmed across components
The updated CSS selector in
drawer.vue
is consistent with the implementation inmodal.vue
, ensuring that whenappendToMain
is true, the component is correctly appended to the designated DOM element. This improved specificity is essential for proper keep-alive functionality.
- File:
packages/@core/ui-kit/popup-ui/src/drawer/drawer.vue
(lines 130-132)- Observation: The selector
>div:not(.absolute)>div
is identical to the one used inmodal.vue
, confirming consistency.playground/src/router/routes/modules/examples.ts (2)
215-217
: AddedkeepAlive
property to fix modal state persistence.This change enables component caching for the
ModalExample
route, which is necessary for maintaining the state of modals when navigating away and returning to this route.
225-227
: AddedkeepAlive
property to fix drawer state persistence.This change enables component caching for the
DrawerExample
route, which is necessary for maintaining the state of drawers when navigating away and returning to this route.playground/src/views/examples/modal/in-content-demo.vue (5)
2-3
: Added necessary import for reactive state management.The import of
ref
is required for the new reactive variable introduced below.
6-6
: Added Input component import for the state persistence test.The Input component is now imported to be used as a test case for the keepAlive functionality.
8-9
: SetdestroyOnClose: false
to maintain component state.This is a crucial change for the fix. By setting
destroyOnClose: false
, the modal's DOM will be preserved when closed, allowing the keepAlive functionality to work correctly.
18-18
: Added reactive variable for testing state persistence.This variable will be used to demonstrate and test that the modal state is properly preserved when navigating away and back.
27-28
: Added Input component for testing keepAlive functionality.This Input component provides a clear way to verify that the modal state is preserved, as any text entered should remain when returning to the modal after navigation.
packages/@core/ui-kit/popup-ui/src/modal/modal.vue (1)
174-178
: Updated element targeting for better DOM placement.The selector has been refined to more precisely target the correct container element, which is essential for proper functioning of keepAlive when using
appendToMain
.
* 修复弹窗和抽屉 `appendToMain` 时且启用`keepAlive` 时未能正确缓存的问题
dd3fc62
to
befb338
Compare
Description
appendToMain
时且启用keepAlive
时未能正确缓存的问题fix: #5665
Type of change
Please delete options that are not relevant.
pnpm-lock.yaml
unless you introduce a new test example.Checklist
pnpm run docs:dev
command.pnpm test
.feat:
,fix:
,perf:
,docs:
, orchore:
.Summary by CodeRabbit