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

Add Svelte demo project #9

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

icheered
Copy link
Contributor

@icheered icheered commented Aug 6, 2023

No description provided.

@RReverser
Copy link
Collaborator

Nice! My main concern is that this is quite a lot of code to keep & potentially maintain in a repo that - as you noticed before - isn't actively maintained.

It looks like a full-featured app. To be fair, so was the original demo, but there the app was the point of the showcase / article.

Now that the library is published to npm, it might best to publish the Svelte app to a separate repo with its own Github Pages or other kind of deployment, and just put a link in the README here instead?

@icheered
Copy link
Contributor Author

icheered commented Aug 7, 2023

I understand your concern, this simple demo quickly grew quite large since I wanted to show the 'right' way of using the package.

I'm thinking I should probably publish this separately, and instead put what I previously had as the 'quick example of using the package' (in the PR readme) as the svelte demo. We could have a 'Cool things made with this package' section and link it to the project?

I'll try to finish this soon but I got kind of stuck with the connect/disconnect issue mentioned in #8. I'll try to see what I can do!

@RReverser
Copy link
Collaborator

but I got kind of stuck with the connect/disconnect issue

Without changing the API the solution is pretty simple - just create a new Camera in the same place you do .connect(). What I said there is just that we should change the API to make it harder to misuse, but for the demo you can work with what you have right now as well.

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