Skip to content
This repository has been archived by the owner on Jun 17, 2021. It is now read-only.

Upgrade to styled-components v4.2.0 #374

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/components/CreateNewProjectWizard/MainPane.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @flow
import React, { PureComponent, Fragment } from 'react';
import { Spring, animated } from 'react-spring';
import { Spring, animated, interpolate } from 'react-spring';
import styled from 'styled-components';

import FormField from '../FormField';
Expand Down Expand Up @@ -146,6 +146,7 @@ class MainPane extends PureComponent<Props> {
} = this.props;

const { lastIndex, steps } = this.renderConditionalSteps(currentStepIndex);
console.log(currentStepIndex);
melanieseltzer marked this conversation as resolved.
Show resolved Hide resolved
return (
<Fragment>
<Spring
Expand Down Expand Up @@ -191,9 +192,9 @@ class MainPane extends PureComponent<Props> {
}
}

const Wrapper = animated(styled.div.attrs(props => ({
const Wrapper = animated(styled.div.attrs(({ translateY }) => ({
style: {
transform: `translateY(${props.translateY}px)`,
transform: interpolate([translateY], y => `translateY(${y}px)`),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortunately this interpolate change isn't producing quite the right result... it's still sticking to the top and not having the correct offset animation. The correct behavior has offset at 50 at first, and when moving to next step, offset becomes 0 and content shifts up smoothly:

But I dug into it a little more and I think I found the solution! Remove the native prop on Spring:

<Spring
  from={{
    offset: currentStepIndex === 0 ? 0 : 50,
  }}
  to={{
    offset: currentStepIndex === 0 ? 50 : 0,
  }}
  // remove
  native
>

And then reset this to the original code:

const Wrapper = animated(styled.div.attrs(({ translateY }) => ({
  style: {
    transform: `translateY(${translateY}px)`,
  },
}))`
  height: 75vh;
  will-change: transform;
`);

This fixes it for me :) I'm not really sure what native is or why it's here, do you know?

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah, OK. The native prop is there for better performance and we should keep it. docs about native prop.

Maybe inlining the translate into a style prop in MainPane could help. I'll have a look later today.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@melanieseltzer Please have a look at the animation. It should be OK now. I'm not sure why animation(styled.div) is not working - maybe I'll create a minimal demo in a Codesandbox to see if it's happening there too.
Could be an issue with Styled-components or React-spring because animation wrapping is recommended in React-Spring docs.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I've created a Codesandbox with it - seems like the style is not added on the element but I'm not sure why.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice, the updated code works 👍 Not sure why it's doing this either. I noticed some more animations not firing so I went and updated them using the way you've outlined, could you double check the logic? Feel free to modify if there's a more elegant way to write these.

},
}))`
height: 75vh;
Expand Down