Skip to content

Commit bde6cef

Browse files
SanderKnauffGaurav0
authored andcommitted
Refactor bs-alert to remove @localcopy and avoid mutation after consumption error in Ember canary (ember-bootstrap#2027)
* Refactor bs-alert to remove @localcopy The `@localCopy` decorate caused issues on Ember canary builds. By refactoring this component to use pure Ember reactivity we can get rid of the decorator. * Allow re-enabling of alert using property after hiding it by clicking
1 parent 4f2213e commit bde6cef

File tree

3 files changed

+45
-17
lines changed

3 files changed

+45
-17
lines changed

addon/components/bs-alert.hbs

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
class="{{unless this.hidden "alert"}} {{if this.fade "fade"}} {{if this.dismissible "alert-dismissible"}} {{bs-type-class "alert" @type}} {{if this.showAlert (if (macroCondition (macroGetOwnConfig "isNotBS3")) "show" "in")}}"
33
...attributes
44
{{did-update this.showOrHide this._visible}}
5+
{{did-update this.updateVisibility @visible}}
56
>
67
{{#unless this.hidden}}
78
{{#if this.dismissible}}

addon/components/bs-alert.js

+16-17
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import { later } from '@ember/runloop';
55
import usesTransition from 'ember-bootstrap/utils/decorators/uses-transition';
66
import deprecateSubclassing from 'ember-bootstrap/utils/deprecate-subclassing';
77
import arg from 'ember-bootstrap/utils/decorators/arg';
8-
import { localCopy } from 'tracked-toolbox';
98

109
/**
1110
Implements [Bootstrap alerts](http://getbootstrap.com/components/#alerts)
@@ -74,12 +73,6 @@ export default class Alert extends Component {
7473
@tracked
7574
hidden = !this.visible;
7675

77-
/**
78-
* This is an unfortunate duplication of the previous property, but this is untracked to avoid causing the dreaded "same computation" assertion in GlimmerVM when reading a tracked property before setting it.
79-
* @private
80-
*/
81-
_hidden = !this.visible;
82-
8376
/**
8477
* This property controls if the alert should be visible. If false it might still be in the DOM until the fade animation
8578
* has completed.
@@ -92,15 +85,16 @@ export default class Alert extends Component {
9285
* @default true
9386
* @public
9487
*/
95-
@arg
96-
visible = true;
88+
get visible() {
89+
return this._visible ?? true;
90+
}
9791

9892
/**
9993
* @property _visible
10094
* @private
10195
*/
102-
@localCopy('visible')
103-
_visible;
96+
@tracked
97+
_visible = this.args.visible;
10498

10599
/**
106100
* Set to false to disable the fade out animation when hiding the alert.
@@ -114,7 +108,7 @@ export default class Alert extends Component {
114108
fade = true;
115109

116110
get showAlert() {
117-
return this._visible && this.args.fade !== false;
111+
return this.visible && this.args.fade !== false;
118112
}
119113

120114
/**
@@ -180,7 +174,7 @@ export default class Alert extends Component {
180174
* @private
181175
*/
182176
show() {
183-
this.hidden = this._hidden = false;
177+
this.hidden = false;
184178
}
185179

186180
/**
@@ -191,7 +185,7 @@ export default class Alert extends Component {
191185
* @private
192186
*/
193187
hide() {
194-
if (this._hidden) {
188+
if (this.hidden) {
195189
return;
196190
}
197191

@@ -200,24 +194,29 @@ export default class Alert extends Component {
200194
this,
201195
function () {
202196
if (!this.isDestroyed) {
203-
this.hidden = this._hidden = true;
197+
this.hidden = true;
204198
this.args.onDismissed?.();
205199
}
206200
},
207201
this.fadeDuration
208202
);
209203
} else {
210-
this.hidden = this._hidden = true;
204+
this.hidden = true;
211205
this.args.onDismissed?.();
212206
}
213207
}
214208

215209
@action
216210
showOrHide() {
217-
if (this._visible) {
211+
if (this.visible) {
218212
this.show();
219213
} else {
220214
this.hide();
221215
}
222216
}
217+
218+
@action
219+
updateVisibility() {
220+
this._visible = this.args.visible;
221+
}
223222
}

tests/integration/components/bs-alert-test.js

+28
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,34 @@ module('Integration | Component | bs-alert', function (hooks) {
150150
assert.dom('.alert').hasClass('alert-success', 'alert has type class');
151151
});
152152

153+
test('alert can be made visible when setting visible=true after manually dismissing it', async function (assert) {
154+
this.set('visible', true);
155+
156+
this.set('handleOnDismissed', () => {
157+
this.set('visible', false);
158+
});
159+
160+
await render(hbs`<BsAlert
161+
@type='success'
162+
@fade={{false}}
163+
@visible={{this.visible}}
164+
@onDismissed={{this.handleOnDismissed}}
165+
>
166+
Test
167+
</BsAlert>`);
168+
169+
assert.dom('.alert').exists('alert has alert class');
170+
assert.dom('button.close').exists({ count: 1 }, 'alert has close button');
171+
172+
await click('button.close');
173+
assert.dom('.alert').doesNotExist('alert has no alert class');
174+
assert.dom('*').hasText('', 'alert has no content');
175+
176+
this.set('visible', true);
177+
178+
assert.dom('.alert').exists('alert has alert class');
179+
});
180+
153181
test('dismissing alert does not change public visible property (DDAU)', async function (assert) {
154182
this.set('visible', true);
155183
await render(hbs`<BsAlert @type="success" @visible={{this.visible}} @fade={{false}}>Test</BsAlert>`);

0 commit comments

Comments
 (0)