Skip to content

Commit

Permalink
Fix FastBoot rendering of opened modals
Browse files Browse the repository at this point in the history
Previously an inline `display: block` (to override a default `display: none`) was set in JavaScript (CSSOM) to not cause strict CSP violations (see #746). But in FastBoot that code would not run (there is no real DOM to set styles to), so an opened modal rendered in FastBoot would show only the backdrop, the actual modals would still be hidden.

This change applies the default BS classes that apply `display: block` in a normal Ember-only way, so they get rendered in FastBoot. That is happening *additionally* to setting inline `display: block`, as I thought that maybe users might have omitted those utility classes from their CSS bundle, in which case modals wouldn't show up at all.
  • Loading branch information
simonihmig committed Aug 23, 2019
1 parent 8b1262e commit 047a1c9
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 6 deletions.
2 changes: 1 addition & 1 deletion addon/components/bs3/bs-modal/dialog.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import ModalDialog from 'ember-bootstrap/components/base/bs-modal/dialog';

export default ModalDialog.extend({
classNameBindings: ['showModal:in']
classNameBindings: ['showModal:in', 'inDom:show']
});
2 changes: 1 addition & 1 deletion addon/components/bs4/bs-modal/dialog.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import ModalDialog from 'ember-bootstrap/components/base/bs-modal/dialog';

export default ModalDialog.extend({
classNameBindings: ['showModal:show'],
classNameBindings: ['showModal:show', 'inDom:d-block'],
centered: false
});
1 change: 1 addition & 0 deletions tests/fastboot/modal-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ module('FastBoot | modal', function(hooks) {

assert.dom('.modal').exists();
assert.dom('.modal').hasClass(visibilityClass());
assert.dom('.modal').isVisible();
assert.dom('.modal-backdrop').hasClass(visibilityClass());
assert.dom('.modal .modal-header .modal-title').hasText('Simple Modal');
assert.dom('.modal .modal-body').hasText('Hi there');
Expand Down
8 changes: 4 additions & 4 deletions tests/integration/components/bs-modal-simple-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,14 @@ module('Integration | Component | bs-modal-simple', function(hooks) {
await render(hbs`{{#bs-modal-simple title="Simple Dialog" fade=false}}Hello world!{{/bs-modal-simple}}`);

assert.dom('.modal').hasClass(visibilityClass(), 'Modal is visible');
assert.equal(this.element.querySelector('.modal').style.display, 'block', 'Modal is visible');
assert.dom('.modal').isVisible();
});

testRequiringTransitions('open modal is immediately shown [fade]', async function(assert) {
await render(hbs`{{#bs-modal-simple title="Simple Dialog"}}Hello world!{{/bs-modal-simple}}`);

assert.dom('.modal').hasClass(visibilityClass(), 'Modal is visible');
assert.equal(this.element.querySelector('.modal').style.display, 'block', 'Modal is visible');
assert.dom('.modal').isVisible();
});

test('open property shows modal', async function(assert) {
Expand All @@ -93,7 +93,7 @@ module('Integration | Component | bs-modal-simple', function(hooks) {
run(() => this.set('open', true));

assert.dom('.modal').hasClass(visibilityClass(), 'Modal is visible');
assert.equal(this.element.querySelector('.modal').style.display, 'block', 'Modal is visible');
assert.dom('.modal').isVisible();

run(() => this.set('open', false));
assert.dom('.modal').doesNotExist('Modal is hidden');
Expand All @@ -108,7 +108,7 @@ module('Integration | Component | bs-modal-simple', function(hooks) {

await settled();
assert.dom('.modal').hasClass(visibilityClass(), 'Modal is visible');
assert.equal(this.element.querySelector('.modal').style.display, 'block', 'Modal is visible');
assert.dom('.modal').isVisible();
this.set('open', false);

await settled();
Expand Down

0 comments on commit 047a1c9

Please sign in to comment.