[WIP] scale bounding client rect

[GavinJoyce]

A WIP PR to address #46

TODO:

• Write tests for scaled containers
• Either figure out what's happening with the container or open a separate issue
• Fix failing tests
  • Acceptance | hero 1️⃣
  • Unit | sprite 2️⃣
• Refactor _getScaledBoundingClientRect?
• Add scale option to ember-animated-tools: [wip] add scale control ember-animated-tools#2

This seems seems to fix movement of sprites when in a scaled container:

There still seems to be some issues to solve with containers:

Although these seem to be present on master too:

[GavinJoyce]

@ef4 before I bring this further, perhaps you could sanity check my approach?

[ef4]

Thanks for working on this.

The key question here is what initialBounds is supposed to mean, as an API design decision. If we can automatically account for all situations by building scale into the sprite bounds, that's great. If we can't, it becomes something that Motions are required to understand.

Right now, some motions do account for scaling, and that is probably what is messing up your containers. Their default motion (Resize) is already accounting for scale, so you would need to take that out: see

ember-animated/addon/motions/resize.js

Lines 25 to 26 in 6f517fe

	sprite.initialBounds.width / sprite.initialCumulativeTransform.a,
	sprite.finalBounds.width / sprite.finalCumulativeTransform.a, duration

.

The smallest change that would fix the bug here is making Move understand the cumulative transforms, the same way Resize does. Probably here:

ember-animated/addon/motions/move.js

Lines 27 to 32 in 6f517fe

	{
	let initial = sprite.initialBounds;
	let final = sprite.finalBounds;
	dx = final.left - initial.left;
	dy = final.top - initial.top;
	}

But in general, I agree with the goal of making it so that motions don't need to individually understand the surrounding cumulative transform. In the most general case, you may be animating inside an Element that is also animating its own scale. The way to make that kind of situation tenable is to keep motions thinking in relative terms. That's why initialBounds is relative to your offset parent -- it means you don't need to know about the fact that you may be inside a context that is itself moving. Probably this means initialBounds should be in coordinates local to the inside of the surrounding cumulative transform, so I think you're on the right track.

[ef4]

I think this branch of the if needs to follow the same coordinate system as the other, so your change would need to go here too.

[ef4]

This is now causing us to calculate the cumulative transform twice. I think this line can just become this._finalCumulativeTransform = this.cumulativeTransform, and then it will share the cache.

[GavinJoyce]

Thanks for the feedback @ef4. I've made those changes and almost everything seems to work:

There is an issue in the hero demo, possibly because it animates scale. I don't fully understand the issue yet, please let me know if it's obvious to you:

There are three failing tests, one is related the the hero issue above. The other two seem like they might just need to be updated to test correct values.

[GavinJoyce]

With a margin, hero animates incorrectly:

.hero-detail {
    margin-left: 6rem;
    margin-top: 6rem;
}

When I remove the .hero-detail margin, it works as expected:

[GavinJoyce]

I rebased and the hero animation seems to be working correctly now:

[GavinJoyce]

@ef4 I'm having a little trouble determining if the three failing tests are due to a bug with scaling or if the tests need to be changes now that we handle scaled containers. Perhaps you could let me know if it's obvious to you?