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

feat: adds rotation property #14028

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

brenoliradev
Copy link

I saw the following issue #14024 and give it a shot since it seamed something easy to adopt.

Ping me if the pattern isn't as expected or if I missed anything.

Cheers,

Copy link

changeset-bot bot commented Oct 30, 2024

⚠️ No Changeset found

Latest commit: fdf543c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@MathiasWP
Copy link
Contributor

Shouldn't the rotation be computed from the from state to the to state? It seems like this just sets the rotation to some arbitrary number.

@brenoliradev
Copy link
Author

Yeah. In this case it's just some arbitrary rotation that'll be played using the u as a reference so it'll starts from rotation and ends on 0deg.

About using "from" and "to" I don't think it's doable without much changes since from and to are DOMRect so their props are just height: number; width: number; x: number; y: number; .

But If you think it's better I can swap to something like rotation={from: number, to: number} and I think it'll make it work as you're expecting.

But a question that pops up is that when the animation ends svelte does removes it's style so I think that's somewhat counterintuitive to have an end another than "0" which would be the "default" rotate.

@Rich-Harris
Copy link
Member

Thank you but this doesn't really solve the feature request. The goal is to be able to morph arbitrary elements into each other based on their rotations (and sizes and positions and whatever else) — you shouldn't have to manually specify a rotation any more than you have to specify scale or translate. Figuring that stuff out is what the function is for.

To do it reliably will be tricky — it should account for the current transform matrix of each element (accounting for all their ancestor elements) and use matrix decomposition to compute a sensible set of transforms. If options are involved they should be to do things like increase the scale at the midpoint (for a 'bump' effect) or ensure that the rotation is clockwise/anticlockwise or add an extra spin etc.

@brenoliradev
Copy link
Author

Oh. Now I get it.

I totally missed what was expected from the rotation behavior. I'll dedicate more time to this issue.

Thanks!

@brenoliradev
Copy link
Author

That's probably my best shot. I'm feeling kinda stucked rn.

Tried to achieve it without changing that much the current structure. Would love to recieve pointers/feedback.

Cheers,

Reproducible playground: https://svelte.dev/playground/6e2424425ad44ae3968707b755f34266?version=5.1.9

@brenoliradev brenoliradev force-pushed the feat/rotate_on_flip branch 2 times, most recently from ac917fd to ac3c7bb Compare November 1, 2024 09:38
Copy link

pkg-pr-new bot commented Nov 4, 2024

pnpm add https://pkg.pr.new/svelte@14028

commit: a41e301

Co-authored-by: Mathias Picker <48158184+MathiasWP@users.noreply.github.com>
@Rich-Harris
Copy link
Member

preview: https://svelte-dev-git-preview-svelte-14028-svelte.vercel.app/

this is an automated message

var dsx = from.width / to.width;
var dsy = from.height / to.height;

var dx = (from.left + dsx * ox - (to.left + ox)) / zoom;
var dy = (from.top + dsy * oy - (to.top + oy)) / zoom;
var { delay = 0, duration = (d) => Math.sqrt(d) * 120, easing = cubicOut } = params;

var rf = Math.atan2(m.m21, m.m11) + Math.atan2(from.bottom - from.top, from.right - from.left);
Copy link
Author

Choose a reason for hiding this comment

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

What about validating this calculation like this:

 function get_initial_angle(transform) {
	if (typeof transform === 'string') return 0

	var m = new DOMMatrixReadOnly(transform);

	return Math.atan2(m.m21, m.m11);
 }
 
	 var rf = get_initial_angle(transform) + Math.atan2(from.bottom - from.top, from.right - from.left);

I think skipping these calculations if transform is none would be cool when multiple elements without initial transforms are animated but I'm unsure how much it would impact complex transformations

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 this pull request may close these issues.

3 participants