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

add layout transitions #1620

Merged
merged 4 commits into from
Sep 12, 2017
Merged

add layout transitions #1620

merged 4 commits into from
Sep 12, 2017

Conversation

homerjam
Copy link

@homerjam homerjam commented Sep 8, 2017

This PR introduces layout transitions (#1519). It would be great to get some feedback/input to get them sorted.

@pi0
Copy link
Member

pi0 commented Sep 8, 2017

Thanks, @homerjam ! Would you please enable some layer transitions in tests/fixtures/with-config? There is no coverage to test template conditions now.

@pi0 pi0 requested a review from Atinux September 8, 2017 17:20
return +to.query.page < +from.query.page ? 'slide-right' : 'slide-left'
},
async asyncData ({ query }) {
const page = +query.page || 1
Copy link
Member

@pi0 pi0 Sep 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice trick to handle nulls :D But doesn't it always resolve to 1 (I mean 0 or null are both falsy values) so maybe can be simplified without +

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup you're almost certainly right : )

I copied the example from route transitions!

@homerjam
Copy link
Author

homerjam commented Sep 8, 2017

I'll absolutely give the tests a go when the feature is nailed. Right now the page transitions do not work - I've been trying all sorts to trigger them. According to the docs nested transitions should be triggered by a key value attached to the target component. In practice I can't seem to get it to work?

@homerjam
Copy link
Author

homerjam commented Sep 9, 2017

Hi @pi0 I've added the layoutTransition option to the with-config tests. Hopefully this is all ok now?

In future it'd obviously be great to be able to override the layoutTransition from page components but I shouldn't think that would be a breaking change. If you can suggest a way to bind the <transition :name="" :mode=""> in App.vue to a dynamic property (one that we can update from pages) then I'll take a look. For now this covers my needs.

@@ -1,6 +1,8 @@
<template>
<nuxt-error v-if="nuxt.err" :error="nuxt.err"></nuxt-error>
<nuxt-child :key="routerViewKey" v-else></nuxt-child>
<div class="nuxt">
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to wrap these components to fix page transitions ¯_(ツ)_/¯

@@ -1,7 +1,9 @@
<template>
<div id="__nuxt">
<% if (loading) { %><nuxt-loading ref="loading"></nuxt-loading><% } %>
<component v-if="layout" :is="nuxt.err ? 'nuxt' : layout"></component>
<% if (layoutTransition) { %><transition name="<%= layoutTransition.name %>" mode="<%= layoutTransition.mode %>"><% } %>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we should always add the <transition> if we want to be able to add layoutTransition into layouts.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean we should always have <transition> even if no layoutTransition is set (so remove the conditional render from the template)?

Or do you mean the <transition> should be added by the user into layout template? This would present an issue with the way layouts work at the moment. The only resolution would be a programatic transition, similar to the one used by pages in nuxt-child.

Copy link
Member

@Atinux Atinux Sep 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean always transition and have default layout transition (like for pages, so the user can add .layout-enter { ... } in css to add an animation :))

@Atinux
Copy link
Member

Atinux commented Sep 11, 2017

Hi @homerjam

I tried indeed to add layout transition but I had the same result as you, the page transitions were broken too.

Could you also see my review and see why the tests are failing?

@homerjam
Copy link
Author

homerjam commented Sep 11, 2017

@Atinux The page transitions are working now - I spent hours trying different things before finally wrapping the nuxt-child and nuxt-error components which fixed it.

AFAIK the tests are failing because of a separate issue:

basic.generate › /validate?valid=true
  /home/travis/build/nuxt/nuxt.js/test/basic.generate.test.js:110
   109:   const html = window.document.body.innerHTML
   110:   t.true(html.includes('I am valid</h1>'))   
   111: })                                           
  Value is not `true`:

I had a quick look and it seems like that test needs some attention, it's broken running against current dev branch.

@Atinux Atinux merged commit dcf3593 into nuxt:dev Sep 12, 2017
@Atinux
Copy link
Member

Atinux commented Sep 12, 2017

I will do more tests on it, you did an amazing job there @homerjam

@homerjam
Copy link
Author

Awesome! Thanks for merging. Should I alter the conditional <transition> in App.vue?

@Atinux
Copy link
Member

Atinux commented Sep 12, 2017

I'll do it :)

@IamManchanda
Copy link

Any updates on getting this released soon?

@homerjam
Copy link
Author

homerjam commented Oct 20, 2017 via email

@ckpiggy
Copy link

ckpiggy commented Dec 20, 2017

@homerjam I trace the layout transition issue to here.
How should I install nuxt from github?
I run npm install nuxt/nuxt.js --save and run nuxt -p 3001 and there is a message complain about no pages directory.

@homerjam
Copy link
Author

@ckpiggy npm i -S nuxt@next will install 'next' version of nuxt or npm i -S github:nuxt/nuxt.js#dev from github (you can swap dev with a commit hash too).

@homerjam
Copy link
Author

Oh and you probably want to use the starter project for initial setup - that will create pages directory etc

@chasebank
Copy link

Hey @homerjam is there still something special that needs to be done for this after v1.0? I'm having trouble getting this working.

@homerjam
Copy link
Author

homerjam commented Jan 22, 2018 via email

@chasebank
Copy link

chasebank commented Jan 22, 2018

Yeah sorry, somehow I missed that the layout- transition hook just works as is. I was trying to set the layoutTransition in the layout files, like you can do for pages, but then found the layout transition example in the examples directory.

I did notice something though. When using both layout and page transitions, the page-enter hooks still fire during a layout-leave transition. The consequence is the new page is temporarily mounted twice; once in the exiting layout, and a second (the correct one) in the incoming layout. Took me a while to figure out what was going on..

https://pumped-eye.glitch.me/

For now I think I've solved it with

.layout-leave-to .page-enter-active {
  display: none;
}

Why that's happening makes sense, now that I know... but I can't think of any reason it would be an intended behavior. So I wonder if there's not some easy way to fix that by default. That's my first time working with nested transitions.

Edit: Forgot to also ask, am I correct that you cannot set the layout transition in each layout? That's unfortunate if true, because then you can't adjust the transition hooks conditionally. If we can only set it in the nuxt.config, as far as I can tell, there's no way to access the vuex state there? Which means the transition has no awareness of the rest of the app. Very limiting when compared to page transitions.

@homerjam
Copy link
Author

@chasebank could be related to #1737 - I have a very similar issue in one of my projects when using nuxt-child. I think the solution will be to somehow prevent the nested transitions from running - perhaps adding a hook to the nuxt/nuxt-child components to check if there is an active parent transition.

@chasebank
Copy link

Apparently there used to be transition.abort() which may have had some relevance, but looks like it was deprecated.

vuejs/vue-router#295

@lock
Copy link

lock bot commented Nov 1, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Nov 1, 2018
@danielroe danielroe added the 2.x label Jan 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants