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

"Unexpected" output when <portal-target multiple> is paired with <transition-group tag="x"> β€” or just bad interpretation of docs? #388

Open
renatodeleao opened this issue Dec 16, 2022 · 3 comments Β· May be fixed by #390
Assignees
Labels

Comments

@renatodeleao
Copy link

renatodeleao commented Dec 16, 2022

Hey πŸ‘‹ First, thanks for the effort in releasing the official 3.0 version, truly appreciated!

Was playing around with it and notice some strange behaviour when using <transition-group> and the #wrapper slot as suggested in documentation.

<!-- from docs -->
<portal-target name="target">
  <template #wrapper="nodes">
    <transition-group name="fade">
      <component :is="node" v-for="node in nodes" :key="node" />
    </transition-group>
  </template>
</portal-target>

The only difference is that my <transition-group> actually renders an element via the tag prop. When using a plain vue <transition-group> and looping inside, only one wrapper element is used even though transition name classes are applied to each item. Since the slot is named #wrapper I also expected it to render once while the portalized elements looped within. If this is the expected behaviour, then we could maybe explicitly add it to the docs.

Reproduction example

To avoid my project context, created two demos: a basic one and another one where I've used the demo "move transition" from vuejs.org docs

Actual

Using <portal-target multiple> #wrapper slot combined with <transition-group tag="ul" class="container" > outputs one <ul class="container"> per portalized item

Expected

Using <portal-target multiple> #wrapper slot combined with <transition-group tag="ul" class="container" > outputs a single <ul class="container"> wrapping all portalized items

Workaround

Not using <transition-group> tag prop, and move a plain wrapper ul outside of portal-target. A problem with this might be reusable AppTransition components which now require a refactor for handling groups. (My problem πŸ˜›). Still, the problem is still there as it seems that mulitple transition-group fragments are outputted.

Real-world context

I have a FABs (Floating-action-buttons) bottom-right container in my app, and each view can portal its own custom FABs in addition to some default one already present at <portal-target multiple>. Note that this is working with portal@2x

fabs-portal-on-scroll

@LinusBorg LinusBorg self-assigned this Dec 16, 2022
@LinusBorg LinusBorg added bug and removed bug labels Dec 16, 2022
@LinusBorg
Copy link
Owner

I think this is mostly an issue with the docs.

The wrapper slot is intended to wrap each item before rendering them. For a single transition group, the default slot should be used.

@LinusBorg LinusBorg added the docs label Dec 16, 2022
@renatodeleao
Copy link
Author

renatodeleao commented Dec 16, 2022

Hey, thanks for looking at it so quickly! Ok if that's intended, the clarification would be nice to have.

For a single transition group, the default slot should be used.

I don't think it solves the problem πŸ€”, since the default slot is replaced when any portal content is moved to portal-target, so I don't think that would work or am I missing something? Since we cannot wrap the actual <portal-target> with a <transition-group> because it requires that the children be actual elements, can you think of any other "more idiomatic" workaround to handle re-usable transition-components? (besides the one I've described in the issue, which breaks the actual reusability).

@LinusBorg
Copy link
Owner

Hm. Have to take another look this weekend. I thought I made it work this morning but can't reproduce it right now. Will be off until tomorrow, so have another look here maybe sunday, please.

I think you are right though and we need a second kind of slot for this.

@LinusBorg LinusBorg linked a pull request Dec 18, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants