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 should sniff for Teleport #5836

Closed
Fuzzyma opened this issue Apr 30, 2022 · 17 comments · Fixed by #6548
Closed

Transition should sniff for Teleport #5836

Fuzzyma opened this issue Apr 30, 2022 · 17 comments · Fixed by #6548
Assignees
Labels
🍰 p2-nice-to-have Priority 2: this is not breaking anything but nice to have it addressed. has PR A pull request has already been submitted to solve the issue scope: teleport scope: transition ✨ feature request New feature or request

Comments

@Fuzzyma
Copy link

Fuzzyma commented Apr 30, 2022

What problem does this feature solve?

Currently, it's not possible to wrap a teleport by a transition because Component inside <Transition> renders non-element root node that cannot be animated.

While I agree that transition shouldn't start trying to find the first actual element it can apply classes to in custom components, I think that there should be an exception for Teleport.

It's not an uncommon use case that you want to teleport some popover or modal to the body and want to add a transition when it appears or gets removed. I am aware that you can always transition the contents of the stuff you are teleporting but that is less flexible and composable. e.g. a reusable component that always ports itself to the body currently can't be wrapped by a custom transition.

Both components are part of the core library and also don't work as normal components do. So I think it shouldn't be a problem if the transition component sniffs if its child is a teleport and applies the transition to the teleported element instead. Ofc that would require that the teleport only consists of a single root but that is a given (the error message already makes that clear and could be changed for the teleport case: SyntaxError: <Transition> expects exactly one child element or component.)

What does the proposed API look like?

<transition>
  <teleport to="body">
    <div>Whatever</div>
  </teleport>
</transition>
@RoosDev
Copy link

RoosDev commented Aug 17, 2022

Yes I'm agree with you.
I would like to animate a component (like a modal) but inside the component I use a teleport ... so the transition doesn t work.

For me it look's like :

<transition> <ModalWindow> </transition>

and in the component ModalWindow :
<Template> <teleport to=".myWonderfullDiv"> <div>Whatever</div> </teleport> </template>

@klausXR
Copy link

klausXR commented Aug 23, 2022

I'm also interested in this

@edison1105
Copy link
Member

I made a PR. feel free to review.
playground

@edison1105 edison1105 self-assigned this Aug 26, 2022
@haoqunjiang haoqunjiang added scope: teleport has PR A pull request has already been submitted to solve the issue 🍰 p2-nice-to-have Priority 2: this is not breaking anything but nice to have it addressed. labels Mar 16, 2023
@zyxkad
Copy link

zyxkad commented Mar 24, 2023

Hello, could teleport support TransitionGroup? Now I'm doing this, but the animation didn't work

<div class="pinned">
  <TransitionGroup id="id" name="animation-class">
  <TransitionGroup/>
</div>
<div>
  <Teleport :disabled="!shouldPin" v-if="shouldShow"></Teleport>
</div>

@MarkSky
Copy link

MarkSky commented Jul 7, 2023

I made a PR. feel free to review. playground

I just create a project just like yours: Teleport-in-Transition
The had running, but the didn't run.
If I miss anything?

@edison1105
Copy link
Member

@MarkSky The PR is not merged.

@MarkSky
Copy link

MarkSky commented Jul 7, 2023

@MarkSky The PR is not merged.

I know.
I just create a project and use your playground code.
but the is not work in my project.

@edison1105
Copy link
Member

@MarkSky
the version of vue in my playground is based on the PR.

@MarkSky
Copy link

MarkSky commented Jul 7, 2023

@MarkSky the version of vue in my playground is based on the PR.

I understand now, thank you.

@Andreashoj
Copy link

Was there a reason this PR was never merged ?

@vhovorun
Copy link

Still waiting when it'll be released

@renatodeleao
Copy link

renatodeleao commented Jul 15, 2024

Hello, could teleport support TransitionGroup? Now I'm doing this, but the animation didn't work

<div class="pinned">
  <TransitionGroup id="id" name="animation-class">
  <TransitionGroup/>
</div>
<div>
  <Teleport :disabled="!shouldPin" v-if="shouldShow"></Teleport>
</div>

Have the same issue as @zyxkad.

Was trying to migrate a feature that used portal-vue to teleport and noticed that teleported nodes are not "caught" by TransitionGroup when it is the teleport target.

context: A FABs (Floating-action-buttons) bottom-right container in my app, and each view can "teleport" its own custom FABs in addition to some default ones already present.

fabs-demo

The patch made by @edison1105 doesn't seem to fix it. Can this be considered a new issue?

@edison1105
Copy link
Member

@renatodeleao
As you can see, TransitionGroup will not re-render after items change, so the transition is not working.

  <TransitionGroup name="list" tag="ul" id="teleport-target"/>

  <template v-for="item in items" :key="item">
    <teleport to="#teleport-target">
      <Item :key="item">{{ item }}</Item>
    </teleport>
  </template> 

@renatodeleao
Copy link

renatodeleao commented Jul 16, 2024

@renatodeleao As you can see, TransitionGroup will not re-render after items change, so the transition is not working.

  <TransitionGroup name="list" tag="ul" id="teleport-target"/>

  <template v-for="item in items" :key="item">
    <teleport to="#teleport-target">
      <Item :key="item">{{ item }}</Item>
    </teleport>
  </template> 

@edison1105 Here's an updated demo, where it does re-render but the transition still does not apply. The only difference is that instead of loop and render several teleport instances, i moved the loop inside a single teleport instance. (i suspect why the previous implementation doesn't work, but out-of-scope of this discussion).

  <h1>Teleported</h1>
  <TransitionGroup name="list" tag="ul" id="teleport-target" />
  
-  <template v-for="item in items" :key="item">
    <teleport to="#teleport-target">
-      <Item>{{ item }}</Item>
+      <Item v-for="item in items" :key="item">{{ item }}</Item>
    </teleport>
-  </template>

@edison1105
Copy link
Member

@renatodeleao

Here's an updated demo, where it does re-render but the transition still does not apply.

The HTML has changed, but it was not due to a re-render by TransitionGroup. Instead, it resulted from a re-render by Teleport. The HTML displayed within TransitionGroup is added by Teleport, and animations are only triggered when the children of TransitionGroup itself change.

@renatodeleao
Copy link

renatodeleao commented Jul 16, 2024

@renatodeleao

Here's an updated demo, where it does re-render but the transition still does not apply.

The HTML has changed, but it was not due to a re-render by TransitionGroup. Instead, it resulted from a re-render by Teleport. The HTML displayed within TransitionGroup is added by Teleport, and animations are only triggered when the children of TransitionGroup itself change.

So if got it right, what you are suggesting since the beginning is that is not currently possible to declaratively teleport items into a TransitionGroup while keeping transitions working. Applying "force re-render techniques" to TransitionGroup component (like the :key="i" and the likes) after teleporting items won't work as well as there's no "diff".

I understand that there are other ways of implementing the same output, but original goal was to swap portal-vue dependency for teleport.

Thank you for your time to clarify, truly appreciated and hopefully this will be helpful for other community members that might land in this issue. 🙏

EDIT: i've highlighted declaratively because we can "teleport" — in a way — using a any global state solution that can be translated into rendering a list of dynamic component definitions from that state (which is basically recreating portal-vue if we want the same declartive API).

@TheKeymaster
Copy link

TheKeymaster commented Jul 19, 2024

For those needing a workaround, i'm gonna explain the issue I've faced and how I solved it. This is how my component looked at first:

<template>
    <Transition name="fade">
      <Teleport to="body" v-if="isOpen">
        <div class="some-div" @click="isOpen = false">
          <img :src="imageUrl" @load="onLoad"/>
        </div>
      </Teleport>
    </Transition>
</template>

At first the issue I had was that I thought I couldn't change the order, because I had the v-if at my <Teleport>. After another think it turns out that it is possible, when I just add the v-if to my <div> instead, which turns out to make it work.

<template>
    <Teleport to="body">
      <Transition name="fade">
        <div class="some-div" @click="isOpen = false" v-if="isOpen">
          <img :src="imageUrl" @load="onLoad"/>
        </div>
      </Transition>
    </Teleport>
</template>

I know if you have a more complex component that renders multiple divs, this may not be feasable. At that point you may think about refactoring your components. I hope this helped some devs out there!

@github-actions github-actions bot locked and limited conversation to collaborators Aug 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🍰 p2-nice-to-have Priority 2: this is not breaking anything but nice to have it addressed. has PR A pull request has already been submitted to solve the issue scope: teleport scope: transition ✨ feature request New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.