Remove # hash params from SceneViewer source #4004
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JL-Vidinoti
pushed a commit
to vidinoti/model-viewer
that referenced
this pull request
Apr 22, 2024
* remove hash from sceneviewer src url * tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reference Issue
Fixes: #3992
Issue
In some circumstances, when auto generating USDZ and appending # hash parameters to the model
srcpath to configure ARQL, these can conflict with the constructed SceneViewer intent, causing SceneViewer to fail to load the model.Solution
When opening AR to SceneViewer, this update will check the model src path for presence of hash params and remove if needed.
Demo
A variety of tests utilising SceneViewer and Auto USDZ, with a mix of query and hash params, can be seen here:
https://modelviewer-auto-usdz-params.glitch.me
Tested on a variety of Android and iOS devices, but any additional testers would be welcome.
Tests
Added two tests to check that hash params for configuring ARQL are completely removed from the model src path and that it also does not remove any query params that may still be used for SceneViewer configuration.