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

1.x breaks animated-outlet #467

Open
RobbieTheWagner opened this issue May 1, 2022 · 10 comments
Open

1.x breaks animated-outlet #467

RobbieTheWagner opened this issue May 1, 2022 · 10 comments

Comments

@RobbieTheWagner
Copy link
Contributor

Describe the bug
@ef4 gave me a rough implementation of animated-outlet, so we could use this instead of liquid-fire. It was something like:

<AnimatedContainer class="h-full">
  <AnimatedValue
    @duration={{this.duration}}
    @intialInsertion={{true}}
    @key="outlets"
    @rules={{this.rules}}
    @value={{-get-dynamic-var "outletState"}}
    as |outletState|
  >
    {{#-with-dynamic-vars outletState=outletState}}
      <div class="h-full overflow-y-scroll w-full">
        {{outlet}}
      </div>
    {{/-with-dynamic-vars}}
  </AnimatedValue>
</AnimatedContainer>
import Component from '@glimmer/component';

import fade from 'ember-animated/transitions/fade';

import { transitionOptions, transitions } from 'swach/transitions';

export default class AnimatedOutlet extends Component {
  duration = transitionOptions.duration;
  easing = transitionOptions.easing;

  rules({ newItems, oldItems }: { newItems: any[]; oldItems: any[] }): unknown {
    const oldRoute = oldItems[oldItems.length - 1];
    const newRoute = newItems[newItems.length - 1];
    let oldRouteName: string | undefined = undefined;
    let newRouteName: string | undefined = undefined;

    if (oldRoute) {
      oldRouteName = oldRoute.outlets.main.render.name;
    }

    if (newRoute) {
      newRouteName = newRoute.outlets.main.render.name;
    }

    let transition = transitions.find(
      (t) => t.from === oldRouteName && t.to === newRouteName
    );

    if (transition) {
      return transition.use;
    }

    transition = transitions.find(
      (t) => t.to === oldRouteName && t.from === newRouteName
    );

    if (transition) {
      return transition.reverse || transition.use;
    }

    if (oldRouteName !== newRouteName) {
      return fade;
    }
  }
}

This worked great in 0.12.0, but has some weird flashy behavior in 1.x. Any thoughts on what could be going on here or what changes I might need to make?

To Reproduce
As part of your reproduction, please fork the Ember Animated Boilerplate Twiddle then list the steps for the behavior.

Here's the Ember Animated Boilerplate Twiddle: https://ember-twiddle.com/d83c87fb8a7bdfa5a55b9a3c2bb4c2e5?openFiles=templates.application.hbs%2C

Steps to reproduce the behavior:

  1. Visit my Twiddle: '...'
  2. Click on '....'
  3. See error

Expected behavior
A clear and concise description of what you expected to happen.

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • OS: [e.g. iOS]
  • Browser [e.g. chrome, safari]
  • Version [e.g. 22]

Smartphone (please complete the following information):

  • Device: [e.g. iPhone6]
  • OS: [e.g. iOS8.1]
  • Browser [e.g. stock browser, safari]
  • Version [e.g. 22]

Additional context
Add any other context about the problem here.

@SergeAstapov
Copy link
Collaborator

@rwwagner90 would you mind to setup simple github repo with reproduction? I think this will make debug much simpler and we can be sure we look into the same thing.

IMO from first glance, nothing suspicious pops up in terms of v1 vs v0.12

@ef4
Copy link
Contributor

ef4 commented May 1, 2022 via email

@RobbieTheWagner
Copy link
Contributor Author

Yeah, very little in ember-animated itself changed. Maybe ember version matters? With animation timing even browser changes have mattered in the past.

If I downgrade ember-animated to 0.12.0 it works. If I update to 1.x, it breaks, so I don't think the Ember version matters. I have also been getting a ton of issues with embroider and when any embroider packages go above 1.3.0 my build fails. I'm wondering if there is something with updating to a v2 addon and the v1 compatibility layer at play here?

I'll try to throw together an example project sometime or I could give you access to Swach temporarily to debug there. I pasted all the code from the component above as well though, so if you have a project you could drop it in, you should see the fade animation doesn't fade everything out and the elements are jumpy.

@acorncom
Copy link
Contributor

acorncom commented Jul 26, 2023

@RobbieTheWagner we hit the same "flashy behavior" you were discussing when using your outlet approach and upgrading an app from 0.12 to 1.0.0 (no problems on 0.12, issues appear on 1.0.0 though I had to temporarily apply the ember-element-helper patch to get 1.0.0 to work locally). Digging into it further, the "flashy behavior" resolves when I install ember-animation-tools (I'm installing [email protected]). Can you check to see if you see the same behavior on your end?

Digging in further here, it seemingly is resolved or broken by the addition of this line here:
https://github.com/ember-animation/ember-animated-tools/blob/9dc17891570e209eba85ba9d92d851f53977b937/addon/src/components/time-control.js#L6

If I have an app with ember-animated-tools installed and delete the entire time-control component or turn off that particular line, I see the erratic behavior. Yet if the component is part of the build and that particular line is enabled this app works smoothly. Currently guessing that we're either hitting generator issues or somehow are affecting state internal to ember-animated when that task is loaded.

Should have more time to dig in on this tomorrow, but curious if anyone else can replicate what I'm seeing

@acorncom
Copy link
Contributor

Following up here, I built out a reproduction (https://github.com/acorncom/animated-outlet-bug-reproduction) that shows the "flashy behavior" when switching between routes. Installation of ember-animated-tools on its own seems to have no impact on the reproduction app, but if I turn on <AnimatedTools /> (which runs the time-control component, even if it isn’t shown) then the flashy behavior reliably goes away.

This still looks like it is somehow triggered by the use of:

import { task } from 'ember-animated';

which is actually loading up ember-scheduler in ember-animated:

export function task(taskFn: (...args: any[]) => Generator) {
let tp = _computed(function (this: HostObject, propertyName: string) {
return new Task(this, taskFn, tp, propertyName);
}) as unknown as TaskProperty;
Object.setPrototypeOf(tp, TaskProperty.prototype);
return tp as ((proto: any, key: string) => any) & TaskProperty;
}
function _computed(fn: (this: HostObject, propertyName: string) => Task) {
let cp = function (proto: HostObject, key: string) {
if ((cp as any).setup !== undefined) {
(cp as any).setup(proto, key);
}
return (computed(fn) as any)(...arguments);
};
(Ember as any)._setClassicDecorator(cp);
return cp;
}

In the time-control component in ember-animated-tools, as if I keep time-control from running or comment out that task completely then I see flashy behavior, but if time-control is run or the task line is imported then I see the outlet smoothly animating.

  • ember-source: 4.12 with the reproduction app, 4.4.x with the main app

@RobbieTheWagner
Copy link
Contributor Author

@acorncom are you saying dropping a <AnimatedTools /> in the app somewhere magically fixes this? Definitely keen for a workaround.

@acorncom
Copy link
Contributor

@RobbieTheWagner that's what I'm seeing, yes. The panel doesn't even have to be open. So in theory you could hide it with some targeted CSS. Quite interested to hear if it works for you as well

@RobbieTheWagner
Copy link
Contributor Author

I can confirm that this is still a bug and dropping a random <AnimatedTools /> in application.hbs does fix the issue. @SergeAstapov does this give any clues into what the problem might be?

@SergeAstapov
Copy link
Collaborator

@RobbieTheWagner no, would need to dig in to see what may be wrong.

@RobbieTheWagner
Copy link
Contributor Author

I have replaced animated-outlet with ViewTransitions now. Check out this great example https://github.com/tcjr/view-transitions-demo

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

No branches or pull requests

4 participants