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

boxel-motion improvements #1105

Merged
merged 1 commit into from
Mar 28, 2024
Merged

boxel-motion improvements #1105

merged 1 commit into from
Mar 28, 2024

Conversation

lukemelia
Copy link
Contributor

@lukemelia lukemelia commented Mar 20, 2024

  • Add Split View example to boxel-motion test app as a study for the AI Assistant Panel animation
  • Incorporate boxel-motion builds into CI
  • Generalize fill behavior of behaviors rather than special casing StaticBehavior treatment
  • Default StaticBehavior to a duration of 1, since you can now opt into forward fill
    • when using StaticBehavior with SpringBehavior, you don't know how long the animation will be, and you may want a static animation to persist for the duration of the animation
  • Use context-relative bounds for removed sprites

extracted from #1097

Copy link

github-actions bot commented Mar 20, 2024

Test Results

550 tests  ±0   546 ✔️ ±0   8m 36s ⏱️ +39s
    1 suites ±0       4 💤 ±0 
    1 files   ±0       0 ±0 

Results for commit 609aaab. ± Comparison against base commit 24de03d.

♻️ This comment has been updated with latest results.

@lukemelia lukemelia force-pushed the motion-improvements branch 2 times, most recently from 59455b6 to 4b95283 Compare March 20, 2024 20:34
@lukemelia
Copy link
Contributor Author

Screen.Recording.2024-03-20.at.4.35.09.PM.mov

@lukemelia lukemelia requested review from habdelra, ef4 and backspace March 20, 2024 20:57
Copy link
Contributor

@backspace backspace left a comment

Choose a reason for hiding this comment

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

@debugging is great 🤩

@lukemelia
Copy link
Contributor Author

A few other thoughts, so I don't forget them:

  1. Specifying to and from can be confusing. The more common case appears to be using captured to and/or from in combination with an "entrance" value for an Inserted Sprite and an "exit" value for a Removed Sprite
  2. the strategy of cloning destroyed sprites to the orphans div under the animation context can cause problems for nested sprites. Given the structure AnimationContext > Sprite A > Sprite B... for animating Sprite B, Inserted and Kept animations are run with a parent of Sprite A, but Removed animations are run with a parent of the AnimationContext. Perhaps it should be cloned beneath Sprite A instead when Removed?

Copy link
Contributor

@ef4 ef4 left a comment

Choose a reason for hiding this comment

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

This seems fine as an incremental step. But in terms of the vision for how I think boxel-motion is supposed to work, fill doesn't really make sense.

The use case for boxel-motion is that you already have an initial and final DOM and the behaviors explain what happens in between those states. If behaviors are able to alter the final DOM, it seems like something about the architecture is getting muddied.

- Add Split View example to boxel-motion test app as a study for the AI Assistant Panel animation
- Incorporate boxel-motion builds into CI
- Generalize fill behavior of behaviors rather than special casing StaticBehavior treatment
- Default StaticBehavior to a duration of 1, since you can now opt into forward fill
    - when using StaticBehavior with SpringBehavior, you don't know how long the animation will be, and you may want a static animation to persist for the duration of the animation
- Use context-relative bounds for removed sprites
@lukemelia lukemelia force-pushed the motion-improvements branch from 860a1b1 to 609aaab Compare March 28, 2024 17:20
@lukemelia lukemelia merged commit cbfccda into main Mar 28, 2024
16 checks passed
@delete-merged-branch delete-merged-branch bot deleted the motion-improvements branch March 28, 2024 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants