From 2eeb466ceaba30601019fd73140248b385f99b22 Mon Sep 17 00:00:00 2001 From: Brett Burley Date: Sun, 3 Jun 2018 18:23:04 -0700 Subject: [PATCH] Apply visibility by clearing the property. `visibility: visible;` and `visibility:;` have different behaviors -- the former is visible even if a parent container has a `visibility: hidden;`. As a result, if there are nested animated containers, and a parent container uses the explode transition, there will be a ghost of the children that have `visibility: visible` applied since the transition relies on cloning the DOM. --- addon/animate.js | 2 +- tests/acceptance/scenarios-test.js | 21 +++++++++++++++++++ .../scenarios/nested-explode-transition.js | 12 +++++++++++ tests/dummy/app/router.js | 1 + .../scenarios/nested-explode-transition.hbs | 16 ++++++++++++++ tests/dummy/app/transitions.js | 8 +++++++ 6 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 tests/dummy/app/controllers/scenarios/nested-explode-transition.js create mode 100644 tests/dummy/app/templates/scenarios/nested-explode-transition.hbs diff --git a/addon/animate.js b/addon/animate.js index dc112c2f..5c7f27ef 100644 --- a/addon/animate.js +++ b/addon/animate.js @@ -40,7 +40,7 @@ export function animate(elt, props, opts, label) { opts.display = ''; } if (typeof(opts.visibility) === 'undefined') { - opts.visibility = 'visible'; + opts.visibility = ''; } if (opts.progress) { diff --git a/tests/acceptance/scenarios-test.js b/tests/acceptance/scenarios-test.js index 25740cec..2ea702a9 100644 --- a/tests/acceptance/scenarios-test.js +++ b/tests/acceptance/scenarios-test.js @@ -4,6 +4,10 @@ import { injectTransitionSpies } from '../helpers/integration'; import { test } from 'qunit'; import moduleForAcceptance from '../helpers/module-for-acceptance'; +function visibility(selector) { + return window.getComputedStyle(find(selector)[0]).visibility; +} + moduleForAcceptance('Acceptance: Scenarios', { beforeEach() { injectTransitionSpies(this.application); @@ -47,3 +51,20 @@ test('model-dependent transitions are matching correctly', function(assert) { ranTransition(assert, 'toRight'); }); }); + +test('nested transitions with explode properly hide children', function(assert) { + visit('/scenarios/nested-explode-transition'); + andThen(() => click('button:contains(Toggle A/B)')); + andThen(() => { + click('button:contains(Toggle One/Two)'); + later(function() { + assert.equal(find('.child-one-b').length, 2, 'explode transition clones child-one'); + assert.equal(visibility('.child-one-b:first'), 'hidden', 'even nested children are hidden'); + assert.equal(visibility('.child-one-b:last'), 'visible', 'nested children of clone are visible'); + + assert.equal(find('.child-two').length, 2, 'explode transition clones child-two'); + assert.equal(visibility('.child-two:first'), 'hidden', 'original child-two is hidden'); + assert.equal(visibility('.child-two:last'), 'visible', 'cloned child-two is visible'); + }, 50); + }); +}); diff --git a/tests/dummy/app/controllers/scenarios/nested-explode-transition.js b/tests/dummy/app/controllers/scenarios/nested-explode-transition.js new file mode 100644 index 00000000..ba8db819 --- /dev/null +++ b/tests/dummy/app/controllers/scenarios/nested-explode-transition.js @@ -0,0 +1,12 @@ +import Controller from '@ember/controller'; + +export default Controller.extend({ + showOne: true, + showA: true, + + actions: { + toggle(prop) { + this.toggleProperty(prop); + } + } +}); diff --git a/tests/dummy/app/router.js b/tests/dummy/app/router.js index ee94105d..141fe423 100644 --- a/tests/dummy/app/router.js +++ b/tests/dummy/app/router.js @@ -72,6 +72,7 @@ Router.map(function() { this.route('three'); }); this.route('in-test-outlet'); + this.route('nested-explode-transition'); }); }); diff --git a/tests/dummy/app/templates/scenarios/nested-explode-transition.hbs b/tests/dummy/app/templates/scenarios/nested-explode-transition.hbs new file mode 100644 index 00000000..d32de0dd --- /dev/null +++ b/tests/dummy/app/templates/scenarios/nested-explode-transition.hbs @@ -0,0 +1,16 @@ + + + +{{#liquid-if showOne class="nested-explode-transition-scenario"}} +
+ {{#liquid-if showA use="toLeft"}} +
One: A
+ {{else}} +
One: B
+ {{/liquid-if}} +
+{{else}} +
+ Two +
+{{/liquid-if}} diff --git a/tests/dummy/app/transitions.js b/tests/dummy/app/transitions.js index f93553c1..464d4f1d 100644 --- a/tests/dummy/app/transitions.js +++ b/tests/dummy/app/transitions.js @@ -195,6 +195,14 @@ this.transition( }) ); + this.transition( + this.hasClass('nested-explode-transition-scenario'), + this.use('explode', { + pick: '.child', + use: ['toLeft', { duration: 500 }] + }), + ); + this.transition( this.includingInitialRender(), this.outletName('test'),