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 example to toggle between mesh variants #4528

Merged
merged 4 commits into from
Oct 27, 2023
Merged

Added example to toggle between mesh variants #4528

merged 4 commits into from
Oct 27, 2023

Conversation

samaneh-kazemi
Copy link
Collaborator

@samaneh-kazemi samaneh-kazemi commented Oct 24, 2023

Reference Issue

link to glitch:

Copy link
Collaborator

@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.

Nice work, the functionality is great! And thanks for including the Glitch link - very helpful for review.

Since this is a public example, code clarity and simplicity are of the utmost importance - I've put in several pointers, but please look through for any ways you can make it simpler and easier to read.

packages/modelviewer.dev/examples/scenegraph/index.html Outdated Show resolved Hide resolved
packages/modelviewer.dev/examples/scenegraph/index.html Outdated Show resolved Hide resolved
packages/modelviewer.dev/examples/scenegraph/index.html Outdated Show resolved Hide resolved
packages/modelviewer.dev/examples/scenegraph/index.html Outdated Show resolved Hide resolved
packages/modelviewer.dev/examples/scenegraph/index.html Outdated Show resolved Hide resolved
packages/modelviewer.dev/examples/scenegraph/index.html Outdated Show resolved Hide resolved
packages/modelviewer.dev/examples/scenegraph/index.html Outdated Show resolved Hide resolved
Copy link
Collaborator

@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.

Looks great, just one small update needed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I just made exactly this mistake - you need to also add this file to ci-before-deploy.sh or else the public example page won't be able to find it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmm wouldn't this cover it?
shared-assets/models/. \

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh shoot, you're right! Thanks.

@elalish elalish merged commit 741a18a into master Oct 27, 2023
7 checks passed
@elalish elalish deleted the meshexample branch October 27, 2023 23:50
JL-Vidinoti pushed a commit to vidinoti/model-viewer that referenced this pull request Apr 22, 2024
* Added example to toggle between mesh variants

* Fixed the alt

* Addressed some of the comments including changing updateAlpha and making options declerative

* simplify code and address all comments
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.

None yet

2 participants