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

Flip animations don't properly account for scale #6657

Closed
Rich-Harris opened this issue Aug 16, 2021 · 3 comments · Fixed by #6658
Closed

Flip animations don't properly account for scale #6657

Rich-Harris opened this issue Aug 16, 2021 · 3 comments · Fixed by #6658

Comments

@Rich-Harris
Copy link
Member

Rich-Harris commented Aug 16, 2021

Describe the bug

When using the animate:flip directive, the resulting transform CSS accounts (incorrectly in some cases, it turns out) for translation but not scaling.

It can easily be worked around, since you can provide your own flip function...

expand to see the function
import { cubicOut } from 'svelte/easing';

export function betterflip(node, { from, to }, params) {
  const style = getComputedStyle(node);
  const transform = style.transform === 'none' ? '' : style.transform;

  const [ox, oy] = style.transformOrigin.split(' ').map(parseFloat);
  const dx = (from.left + from.width * ox / to.width) - (to.left + ox);
  const dy = (from.top + from.height * oy / to.height) - (to.top + oy);

  const {
    delay = 0,
    duration = (d) => Math.sqrt(d) * 120,
    easing = cubicOut
  } = params;

  return {
    delay,
    duration: typeof duration === 'function' ? duration(Math.sqrt(dx * dx + dy * dy)) : duration,
    easing,
    css: (t, u) => {
      const x = u * dx;
      const y = u * dy;
      const sx = t + u * from.width / to.width;
      const sy = t + u * from.height / to.height;
      
      return `transform: ${transform} translate(${x}px, ${y}px) scale(${sx}, ${sy});`;
    }
  };
}

...but there's no reason not to have a better function included in the library.

There are other improvements I'd like to make to animations in the long run (the fact that they can only be applied to direct children of a keyed each is a nuisance — in the REPL below it means you can't have e.g. a box with a grey border behind each coloured box, and you can't apply scaling to the text that counteracts the scaling of the parent elements), but this is some good low-hanging fruit in the meantime.

Reproduction

https://svelte.dev/repl/0c9972ed9bc14af98e7045221f77883e?version=3.42.1. Click the boxes, shuffle the order. Toggle the checkbox to activate a better flip function that does take account of scale.

Logs

No response

System Info

visible in REPL with version 3.42.1

Severity

annoyance

@Conduitry
Copy link
Member

Is this the same as #5996 / will that also be addressed by the same PR?

@simeydotme
Copy link
Contributor

As mentioned in the MR comment this regresses a previous fix for FLIP animations occurring inside a scaled ancestor.

Here's a REPL demonstrating the better flip animation failing to account for a scaled ancestor.
https://svelte.dev/repl/bc936592fbd24977ac21dce48e3b7e3d?version=3.42.1

@rchrdnsh
Copy link

rchrdnsh commented Sep 9, 2022

it seems that flip animations still do not account for scale?

Here is a REPL:

https://svelte.dev/repl/1236f2b67e7e4211b12758079c89afe2?version=3.50.1

...where one box is moved on a grid and expanded to fit two columns instead on one while being moved...The issue is that the borders, which have a radius of 8px are stretched and squished while FLIPing back as well as the contents inside the box, which is the word 'box' in this case.

It is my understanding that when FLIPing in general these kinds of things are accommodated, as would be the case with a library like framer-motion...

So, I don't know if this is a limitation of FLIPing in Svelte, or if these is a way to accomplish this in svelte using other means...

In any case, I wanted to bring it to the core teams attention. I also asked on the discord and will update here if I get anything from there :-)

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

Successfully merging a pull request may close this issue.

4 participants