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

Restoring multimarkers examples #419

Merged
merged 6 commits into from
Apr 23, 2022
Merged

Conversation

kalwalt
Copy link
Member

@kalwalt kalwalt commented Apr 20, 2022

⚠️ All PRs have to be done versus 'dev' branch, so be aware of that, or we'll close your issue ⚠️

What kind of change does this PR introduce?

It try to restore multimarkers examples.
Can it be referenced to an Issue? If so what is the issue # ?
There is not a specific issue but it is part of #402
How can we test it?

Test the learner.html and player.html inside three.js/examples/multi-markers/examples/
Summary

This PR not improve the multi-markers examples, it simply fix the url paths for the examples.
Does this PR introduce a breaking change?
No at all.
Please TEST your PR before proposing it. Specify here what device you have used for tests, version of OS and version of Browser
Tested on desktop Ubuntu 20.04, the learner and player seems to work, But i can't assure and i will not work on that feature for now.
Other information
This feature was under development in the old repository by the original creator, we do a minimal restore but i personally i have not time to continue on this.

@kalwalt kalwalt added the enhancement New feature or request label Apr 20, 2022
@kalwalt
Copy link
Member Author

kalwalt commented Apr 20, 2022

@nickw1 i merge this soon there is not needs to test (i have tested), as mentioned at the top this was under development and i'm not sure how we can continue. At least we can make this minimal restore but PR's are welcome to improve it!

@nickw1
Copy link
Collaborator

nickw1 commented Apr 22, 2022

@kalwalt seems fine to merge in that case. I don't know so much about marker-based so I doubt I can work on it myself, but yes PRs would be welcome!

@kalwalt
Copy link
Member Author

kalwalt commented Apr 23, 2022

latest commit f5ae7eb should fix #411

@kalwalt kalwalt merged commit 1fbb0a3 into dev Apr 23, 2022
@kalwalt
Copy link
Member Author

kalwalt commented Apr 23, 2022

@nickw1 if you agree i will make a new release soon when i have time. Probably we can make a 3.4.0 release now?

@kalwalt kalwalt deleted the restoring-multimarkers-examples branch April 23, 2022 20:53
@nickw1
Copy link
Collaborator

nickw1 commented Apr 24, 2022

@kalwalt sounds good. I might just add some READMEs to the various location-based APIs, if that's OK - to ensure users are clear what is what. I think I will encourage people to use the new-location-based version if they want to use location-based with A-Frame, and then fall back to the older version if they have problems.

@nickw1
Copy link
Collaborator

nickw1 commented Apr 26, 2022

@kalwalt OK have added the READMEs so feel free to release when you have time.

@kalwalt
Copy link
Member Author

kalwalt commented Apr 26, 2022

@nickw1 i will do it soon!

@nickw1
Copy link
Collaborator

nickw1 commented Apr 27, 2022

@kalwalt ok great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants