fix(BToast): close BToast correctly if modelValue is changed from number to false #2745
Conversation
|
|
|
Warning Rate limit exceeded@xvaara has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 52 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
""" WalkthroughThe changes update the BToast component's watcher to handle transitions of Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ToastController
participant BToast
User->>ToastController: Call destroy()
ToastController->>BToast: Call hide()
BToast->>BToast: modelValue changes (number -> false)
BToast->>BToast: stop countdown
BToast->>User: Toast closes
Assessment against linked issues
Poem
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. 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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/bootstrap-vue-next/src/plugins/toastController/index.ts (1)
98-99: Consider passing an explicit trigger tohide()Inside the
destroy()method you now callthis.hide()fromnextTick.
While this works, omitting thetriggerargument means downstream listeners cannot distinguish a normal hide operation from a destruction flow. Supplying a descriptive string (e.g."destroy") preserves the intent and keeps the public event contract symmetric with other callers (hide('close'), etc.).- nextTick(() => { - this.hide() + nextTick(() => { + // Mark that this hide originates from destroy() + this.hide('destroy') })packages/bootstrap-vue-next/src/components/BToast/BToast.vue (1)
246-267: Watch handler now covers number→false transition – looks correctThe added
oldValueparameter and the extra guard (lines 259-266) cleanly stop the countdown whenmodelValueflips from a numeric timeout tofalse. Logic is sound and does not interfere with existing branches.
Minor style nit:typeof newValue === 'boolean' && newValue === falsecan be shortened tonewValue === falsewithout changing behaviour.- if ( - typeof newValue === 'boolean' && - newValue === false && + if ( + newValue === false && typeof oldValue === 'number' && oldValue > 0 ) { stop() }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/bootstrap-vue-next/src/components/BToast/BToast.vue(2 hunks)packages/bootstrap-vue-next/src/plugins/toastController/index.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build
- GitHub Check: test-unit
- GitHub Check: test-lint
commit: |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bootstrap-vue-next/src/components/BModal/BModal.vue(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: test-unit
There was a problem hiding this comment.
Pull Request Overview
This pull request fixes an issue with BToast not closing properly when its modelValue transitions from a number to false. The changes include updating the toast plugin to use a hide method with a destroy trigger, modifying the BToast watch to handle false values, and improving modal z-index initialization.
- Updated toast plugin to call hide('destroy') instead of manually setting modelValue.
- Enhanced BToast to stop the countdown when transitioning to false.
- Refined BModal to ensure document.body is referenced safely.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/bootstrap-vue-next/src/plugins/toastController/index.ts | Replaces manual state update with this.hide('destroy') call. |
| packages/bootstrap-vue-next/src/components/BToast/BToast.vue | Adds a watch branch for modelValue false transitions and debugging log. |
| packages/bootstrap-vue-next/src/components/BModal/BModal.vue | Adjusts z-index retrieval to include a safeguard for document.body. |
Comments suppressed due to low confidence (1)
packages/bootstrap-vue-next/src/plugins/toastController/index.ts:98
- Ensure that 'this' correctly refers to the expected toast instance in this context. If there is any ambiguity, consider explicitly using the toast instance (e.g., toast.hide('destroy')).
this.hide('destroy')
| @@ -256,6 +256,10 @@ watch(modelValue, (newValue) => { | |||
| if (typeof newValue === 'number' && newValue === 0) { | |||
| stop() | |||
| } | |||
| if (newValue === false && typeof oldValue === 'number' && oldValue > 0) { | |||
| console.log('stop', newValue, oldValue) | |||
There was a problem hiding this comment.
Remove the debugging console.log statement or replace it with proper logging if necessary.
Copilot uses AI. Check for mistakes.
fixes #2743
fixes #2736
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:
CHANGELOGis 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
Summary by CodeRabbit