Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transition #98

Closed
madmoizo opened this issue Jan 7, 2018 · 13 comments · Fixed by #145
Closed

Transition #98

madmoizo opened this issue Jan 7, 2018 · 13 comments · Fixed by #145

Comments

@madmoizo
Copy link

madmoizo commented Jan 7, 2018

Hi,

vue: 2.5.13
portal-vue: 1.3.0-beta.0

I have a basic usage of portal-vue for modal display (because sadly (or not) z-index is relative)

<portal-target name="modal" transition="modal-transition"></portal-target>

<portal to="modal">
    <router-view></router-view>
</portal>

I got the following breaking error (there is only one modal at a time)
[Vue warn]: <transition-group> children must be keyed: <>
I fixed it with slim props but is it a normal behavior ?

Other thing, transition works fine on the initial state if used like this:

<portal to="modal">
    <router-view>
           /* 
             <transition name="modal-transition">
                    <div class="modal"></div>
            </transition>
          */
    </router-view>
</portal>

But the leave part of the animation is ignored.
Is it possible to get the animation duration then clean the portal-target after it ends ?
Because ideally, I see portal-vue as an option. If I have to move the transition part of a component to get it works, its behavior with or without portal will be different and it's a problem.

Thanks for your work on this lib!

@LinusBorg
Copy link
Owner

is it a normal behavior?

Yes it is. Without the slim prop, a transition-group is used, and as the error states, it's children need a key.

For the second problem I would need to see some actual code that I can run.

@madmoizo
Copy link
Author

madmoizo commented Jan 7, 2018

Thanks for the quick answer. I'll update with a demo asap

@madmoizo
Copy link
Author

madmoizo commented Jan 8, 2018

Harder than expected to reproduce in a codepen, I'll create a dedicated repo

@LinusBorg LinusBorg added the needs reproduction We need a runnable code example to verify your bug report label Jan 12, 2018
@madmoizo
Copy link
Author

madmoizo commented Jan 15, 2018

@LinusBorg
Copy link
Owner

thanks, will take a look in the coming days. I'm a bit short on time at the moment, unfortunately.

@tinymachine
Copy link

tinymachine commented Feb 8, 2018

+1 on the leave part of the transition being ignored (vue: 2.5.17, portal-vue: 1.3.0).

Repo: https://github.com/tinymachine/portal-vue-transition-examples
Demo: https://vue-portal-transition-leave-issue.netlify.com

Thanks for open-sourcing this project!

@kfirba
Copy link

kfirba commented May 6, 2018

The enter transition works just as expected but the leave transition never triggers. Any news on that issue?

@redplant3d
Copy link

Hi @kfirba , it's because of the this.$el.innerHTML = '' in the portal-target.js.
beforeDestroy() { this.unwatch() this.$el.innerHTML = '' },
@LinusBorg: Does the removal of the innHtml-line have any sideeffects?

@LinusBorg
Copy link
Owner

Good catch. I remember I put it there for some reason, but forgot what it was, to be perfectly honest.

Maybe it might not be necessary (any longer)

@kfirba
Copy link

kfirba commented Jun 4, 2018

@redplant3d Good catch.

@LinusBorg Any plans to address this? Do you need us to check something?

@LinusBorg
Copy link
Owner

Yes I'm planning to address this, but I'm still struggling to make time for this project sind February. Was moving, had a huge project at work and so on.

My schedule will relax considerably around the end of June, but before that' i won't be able to much.

If you want to submit a PR with this line removed and passing tests, that would be appreciated.

@LinusBorg LinusBorg added bug and removed needs reproduction We need a runnable code example to verify your bug report labels Jul 4, 2018
@thomasaull
Copy link

thomasaull commented Aug 16, 2018

+1 for this, I just ran into the same issue :)

Oh and quick update: It works for me if I add the slim option to the portal-target

LinusBorg added a commit that referenced this issue Sep 2, 2018
LinusBorg added a commit that referenced this issue Sep 2, 2018
* fix: #98

* fix: #131

* make tests run again
@tinymachine
Copy link

tinymachine commented Sep 21, 2018

@LinusBorg: Hope it's not poor form to tag you on this; just want to make sure you were aware of this issue since 1.4.0-beta.1 is supposed to fix this transition issue, but, in my testing at least, it seems not to.

I might be missing something, but I updated to the 1.4.0-beta.1 version and I'm still seeing the issue of the leave transitions not displaying:

1.4.0-beta.1 example
1.3.0 example
(links to repos appear in the examples)

Any help you can offer would be great -- thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants