fix(alert): dismissed event not emitted if dismissed-event-countdown is listened (closes #2967)
#2968
Conversation
Should be enough to fix bootstrap-vue#2967
Codecov Report
@@ Coverage Diff @@
## dev #2968 +/- ##
======================================
Coverage ? 92.06%
======================================
Files ? 205
Lines ? 3251
Branches ? 960
======================================
Hits ? 2993
Misses ? 190
Partials ? 68
Continue to review full report at Codecov.
|
| @@ -63,9 +63,10 @@ export default { | |||
| dismiss() { | |||
| this.clearCounter() | |||
| if (typeof this.show === 'number') { | |||
| this.$emit('dismiss-count-down', 0) | |||
There was a problem hiding this comment.
If someone clicks the dismiss close button, the countdown timer value will not be emitted as 0 (and some users rely on this event emitting 0 when dismissed early.
There was a problem hiding this comment.
you should check first if this.dismissCountDown is greater than 0, and if so, emit 'dismiss-count-down with a value of 0
There was a problem hiding this comment.
That's the original code - I've just added an extra event to emit dismissed.
I do agree though, but there is no this.dismissCountDown - it's a local variable in the showChanges method.
I can add another parseInt on this.show and then if it's greater than 0, emit the event?
There was a problem hiding this comment.
Ah right... in the revamped code I was working on for BAlert, I was switching to setTimeout from setInterval (to simplify the code/logic) and storing the dismissCountDown value in data().
Maybe at line 95 where we defined the variable dismissCountDown, it should be stored in data, and any decrement would be done on the this.dismissCountDown.
There was a problem hiding this comment.
Or, I can update the BAlert code with the new code I was getting ready to implement and see if it fixes the original issue (as it also fixes a few other issues with BAlert)
There was a problem hiding this comment.
@tmorehouse I'm not fussy - I just need to see this fixed as I'm planning on using it heavily.
dismissed event not emitted if dismissed-event-countdown is listened (closes #2967)
src/components/alert/alert.js
Outdated
| @@ -63,9 +63,13 @@ export default { | |||
| dismiss() { | |||
| this.clearCounter() | |||
| if (typeof this.show === 'number') { | |||
| this.$emit('dismiss-count-down', 0) | |||
| if (parseInt(this.show) || 1) > 0 { | |||
There was a problem hiding this comment.
There is a closing bracket missing.
There was a problem hiding this comment.
This is also assuming that the user is v-modeling the show prop, which is not always the case.
It would be better to check the value of this.dismissCountDown to see if it is greater than 0.
There was a problem hiding this comment.
@tmorehouse Well i tried... Still not seeing any reference to this.dismissCountDown and the only reference to dismissCountDown in the entire file is a local variable in the showChanged() method, as per my previous comment.
|
Closing in favour of #2969 |
Description of Pull Request:
This should fix the issue with the incorrect events being emitted when a dismissable alert is being counted down.
Closes #2967.
PR checklist:
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
If yes, please describe the impact:
The PR fulfills these requirements:
devbranch, not themasterbranchfixes #xxxx[,#xxxx], where "xxxx" is the issue number)and adding a new feature, break them into separate PRs if at all possible.
Conventional Commits naming convention (i.e.
"fix(alert): not alerting during SSR render", "docs(badge): Updated pill examples, fix typos",
"chore: fix typo in docs", etc). This is very important, as the
CHANGELOGis generatedfrom these messages.
If new features/enhancement/fixes are added or changed:
package.jsonfor slot andevent changes)
keyboard only users? clickable items should be in the tab index, etc)
If adding a new feature, or changing the functionality of an existing feature, the PR's
description above includes:
suggestion issue first and wait for approval before working on it)