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.

Sign up for GitHub

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

Jump to bottom

Tweenable does not seem to be reusable #133

Closed
tfsJoe opened this issue Oct 15, 2021 · 5 comments
Closed

Tweenable does not seem to be reusable #133

tfsJoe opened this issue Oct 15, 2021 · 5 comments

Comments

@tfsJoe
Copy link

tfsJoe commented Oct 15, 2021

In the documentation here, there is an example of configuring a Tweenable, and calling .tween(). There is a comment that says:

// tweenable.tween() could be called again later
tweenable.tween().then(() => console.log('All done!'))

So, I attempted to reuse a Tweenable, by calling .tween() again. However, the result was, for the duration set in the config, the final value is always used in the render callback. Here is an example - the green div called thing uses a new tween every time, and the red one called thing2 attempts to reuse a tweenable. Based on the comment in the doc, I would expect the red box to go back to y = 0 and animate through y = 500. Instead, after animating from 0 to 500 the first time, the second time it animates from 500 to 500 and just stays put.

<!DOCTYPE html>
<HTML>
    <HEAD>
        <TITLE>Shifty anim lib test</TITLE>
        <SCRIPT type="text/javascript" src="shifty.core.js"></SCRIPT>
        <LINK rel="icon" type="image/png" href="/ui_img/trickyfasticon.png" />
        <STYLE>
#thing {
    position: absolute;
    width: 50px;
    height: 50px;
    background-color: green;
}

#thing2 {
    position: absolute;
    width: 50px;
    height: 50px;
    left: 100px;
    background-color: tomato;
}
        </STYLE>
    </HEAD>
    <BODY class="tight">

        <DIV id="thing" onclick="clickHandler(this).then(()=>console.log('whew'));"> :-) </DIV>

        <DIV id="thing2" onclick="myTweenable.tween();"> :-( </DIV>

        <SCRIPT>
thing = document.getElementById('thing');
thing2 = document.getElementById('thing2');

tween = shifty.tween;
async function clickHandler(element) {
    console.log("ouch!");
    let bounds = element.getBoundingClientRect();
    let myAnim = tween({
        render: ({x}) => {element.style.left = x + 'px'},
        from: {x: bounds.left},
        to: {x: (Math.random() * 600),
        duration: 500,
        easing: 'easeInQuad'}
    });
    await myAnim;
}

myTweenable = new shifty.Tweenable();
myTweenable.setConfig({
    from: {y: 0},
    to: {y: 500},
    duration: 500,
    easing: "easeInQuad",
    render: (state => {
        thing2.style.top = state.y + "px";
        console.log(state.y);
    })
});
        </SCRIPT>
    </BODY>
</HTML>

I've attached a screenshot of the console, with a pink line drawn through the first and second runs:
Screen Shot 2021-10-15 at 6 04 27 PM
@jeremyckahn
Copy link
Owner

Hi @tfsJoe, this is an excellent bug report! Thanks for opening it. I just tried to reproduce this issue in CodePen and am seeing the same behavior: https://codepen.io/jeremyckahn/pen/zYdrNZz?editors=1111

tween doesn't restart a tween from the beginning, although I can see why that would be assumed. Here's the source for that function:

shifty/src/tweenable.js

Lines 334 to 352 in 520ba43

tween(config = undefined) {
if (this._isPlaying) {
this.stop()
}
if (config || !this._config) {
this.setConfig(config)
}
this._pausedAtTime = null
this._timestamp = Tweenable.now()
this._start(this.get(), this._data)
if (this._delay) {
this._render(this._currentState, this._data, 0)
}
return this._resume(this._timestamp)
}

All it really does it perform some housekeeping around timing and then begin playing with its current config state. In your sample code, setConfig is called once at the beginning, so the initial tween works correctly. However, subsequent calls to myTweenable.tween() don't provide a new config object, so the tween simply uses whatever internal config state it has.

I think you can achieve the behavior you want by changing:

myTweenable.tween();

To something like:

// Assumes newStartingPosition returns updated starting position data
myTweenable.tween({ from: newStartingPosition() });

Shifty's current behavior is perhaps not ideal; it should probably restart completed tweens when a config object is not provided. I can keep this issue open to make that change, but I am pretty tied up with other projects at the moment so I'm not sure when I might be able to work on it (though feel free to make a PR!).

Let me know if this helps!

@tfsJoe
Copy link
Author

tfsJoe commented Oct 16, 2021

Thanks for the quick response. I appreciate the workaround, this will work for me. Of course it is up to you as to whether this issue stays open, but if you do decide not to change it I would at least consider this a defect in the documentation - it should probably mention that the configuration needs to be supplied anew.

jeremyckahn added a commit that referenced this issue Oct 18, 2021
@jeremyckahn
[#133] Don't mutate config data on tween completion
b3dc299
@jeremyckahn
Copy link
Owner

@tfsJoe I had a bit of spare time and I think I found a fix for this issue: b3dc299. It was actually just the removal of some code that I'm not quite sure was for to begin with. 😅

I've put up a pre-release version (2.16.0-0): https://unpkg.com/shifty@2.16.0-0/dist/shifty.js

It seems to be working with the sample code you provided: https://codepen.io/jeremyckahn/pen/wvqGWQw Could you give this a look and let me know if it's working the way you expect? If it is, I can release it in 2.16.0.

@tfsJoe
Copy link
Author

tfsJoe commented Oct 18, 2021

I pulled it down and used it in my sample, and reviewed the change. With the caveat that I am new to the codebase, this makes sense to me and results in my expected behavior, so it looks good to me!

@jeremyckahn
Copy link
Owner

Awesome, thanks for checking @tfsJoe! The fix has been released in 2.16.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jeremyckahn @tfsJoe