-
-
Notifications
You must be signed in to change notification settings - Fork 151
fix(BAccordionItem): fix initial modelValue #2663
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(BAccordionItem): fix initial modelValue #2663
Conversation
|
WalkthroughThe change modifies the initialization logic of the Changes
Sequence Diagram(s)sequenceDiagram
participant ParentAccordion
participant AccordionItem
Note over AccordionItem: On component initialization
alt modelValue.value is true
AccordionItem->>ParentAccordion: Check if mode is not 'free'
alt not 'free'
AccordionItem->>ParentAccordion: Set open item to computedId
end
else modelValue.value is false
AccordionItem->>ParentAccordion: Check if open item matches computedId and !initialAnimation
AccordionItem->>AccordionItem: Set modelValue.value accordingly
end
Assessment against linked issues
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 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 (1)
🪧 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 (
|
commit: |
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 (1)
packages/bootstrap-vue-next/src/composables/useShowHide.ts (1)
236-285
: Consider adding the mounted check to the hide function's callback too.While you've added a check in the show function to prevent execution after unmount, a similar check is missing in the hide function's callback.
Consider adding the same safety check in the hide timeout callback:
hideTimeout = setTimeout( () => { + if (!isMounted) return hideTimeout = undefined isLeaving.value = true showRef.value = false options.hideFn?.() if (modelValue.value) { noAction = true modelValue.value = isCountdown ? 0 : false } }, // ... )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bootstrap-vue-next/src/composables/useShowHide.ts
(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (4)
packages/bootstrap-vue-next/src/composables/useShowHide.ts (4)
8-8
: Good addition of lifecycle hook import.Adding the
onUnmounted
import is necessary for the lifecycle management improvements.
113-113
: Good implementation of mount status tracking.The
isMounted
flag enables proper lifecycle management, correctly initialized and set to true when the component is mounted.Also applies to: 115-115
216-216
: Excellent defensive programming in the delayed callback.Adding the mount check prevents state updates after a component is unmounted, which prevents potential errors and memory leaks.
317-323
: Good resource cleanup on component unmount.The
onUnmounted
hook properly:
- Sets the mounted flag to false
- Clears any pending timeouts
- Resets timeout references
This prevents memory leaks and execution of callbacks after component unmount.
fixes #2657
fixes #2656
Describe the PR
A clear and concise description of what the pull request does.
Small replication
A small replication or video walkthrough can help demonstrate the changes made. This is optional, but can help observe the intended changes. A mentioned issue that contains a replication also works.
PR checklist
What kind of change does this PR introduce? (check at least one)
fix(...)
feat(...)
fix(...)
docs(...)
The PR fulfills these requirements:
CHANGELOG
is generated from these messages, and determines the next version type. Pull requests that do not follow conventional commits or do not have an override will be deniedSummary by CodeRabbit