Skip to content

animation support ignore default#964

Open
MakinoharaShoko wants to merge 1 commit into
devfrom
animation-ignore-default
Open

animation support ignore default#964
MakinoharaShoko wants to merge 1 commit into
devfrom
animation-ignore-default

Conversation

@MakinoharaShoko
Copy link
Copy Markdown
Member

No description provided.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying webgal-dev with  Cloudflare Pages  Cloudflare Pages

Latest commit: 8accb4c
Status: ✅  Deploy successful!
Preview URL: https://d23b489d.webgal-dev.pages.dev
Branch Preview URL: https://animation-ignore-default.webgal-dev.pages.dev

View logs

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces an ignoreDefault option across various animation scripts (including changeBg, changeFigure, setAnimation, setTempAnimation, setTransform, and setTransition) to control whether default animation settings are ignored. It updates the stage animation settings interface and refactors the animation timeline logic to support this behavior. Feedback was provided on getAnimationTimeline to explicitly exclude 'scale' and 'position' from the initial pickBy call when writeFullEffect is false, preventing potential side effects from copying nested objects by reference when they are not being animated.

Comment on lines +78 to +80
const originalTransform = { ...pickBy(sourceTransform, (source, key) => unionKeys.has(key)) };
if (unionScaleKeys.size > 0) originalTransform.scale = targetScale;
if (unionPositionKeys.size > 0) originalTransform.position = targetPosition;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

When writeFullEffect is false, pickBy is used to copy properties from sourceTransform based on unionKeys. However, since scale and position are nested objects, copying them directly by reference can lead to unexpected behavior if they are not being animated (i.e., when unionScaleKeys or unionPositionKeys are empty).

By explicitly excluding 'scale' and 'position' from the initial pickBy call, we can ensure they are only added to originalTransform when they are actually being animated.

        const originalTransform = {
          ...pickBy(sourceTransform, (source, key) => unionKeys.has(key) && key !== 'scale' && key !== 'position'),
        };
        if (unionScaleKeys.size > 0) originalTransform.scale = targetScale;
        if (unionPositionKeys.size > 0) originalTransform.position = targetPosition;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant