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

Added the ability to run and combine multiple animations simultaneously #4979

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

mohammadbaghaei
Copy link

Added the ability to run and combine multiple animations simultaneously with appendAnimation and detachAnimation methods, updated documentation, upgrade to three.js r171 and update all dependencies to latest version

Copy link
Contributor

@elalish elalish left a comment

Choose a reason for hiding this comment

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

Wow, this is quite a feature, thank you! To make this easier to review and merge, would you be so kind as to split the dependency updates into a separate PR from the animation work? We should be able to quickly merge the updates, but we'll probably need some back-and-forth on the API change. Honestly, it's looking really good though!

packages/model-viewer/package.json Outdated Show resolved Hide resolved
packages/model-viewer/src/features/animation.ts Outdated Show resolved Hide resolved
packages/modelviewer.dev/data/docs.json Outdated Show resolved Hide resolved
packages/modelviewer.dev/data/docs.json Outdated Show resolved Hide resolved
…ove appendAnimation method, format codes with clang-format style and fix other requested changes in google#4979 pull request
…of appendAnimation function, improve modelviewer.dev styles and revert dependencies version
@mohammadbaghaei
Copy link
Author

Hey there! Here's my rework of that PR message:

👋 Thank you for the helpful feedback! I've made the following changes in this PR:

  • Fixed all the mentioned issues
  • Enhanced modelviewer.dev styles for better presentation, especially in the examples section
  • Updated dependencies in a separate commit
  • Improved appendAnimation and detachAnimation methods to automatically handle invalid parameters
  • Added automatic parameter type conversion - numbers and booleans can now be passed as strings (case-insensitive)
  • For better AR support across different devices (ref: issue AR/MR mode doesn't work on PICO headsets / Make Hit Test API optional #4960), moved hit-test to optionalFeatures

Let me know if any further adjustments are needed! 🙂

@elalish
Copy link
Contributor

elalish commented Dec 30, 2024

Thanks! As this is my last day (see #4975), I'm going to pass this on to one our other maintainers, @diegoteran, who I already talked to about this PR. Looks like a promising feature!

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.

2 participants