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

Documentation: Guide update: Updates to "Basic Scene" Guide #5213

Merged
merged 14 commits into from
Nov 14, 2024

Conversation

diarmidmackenzie
Copy link
Contributor

Description:

2nd of what will in fact be 4 updates to bring external glicth examples up-to-date & into the A-Frame repo, per #5207.

  • Bring code from Glitch into A-Frame Examples
  • Code up-to-date with latest versions
  • Code and documentation match

@diarmidmackenzie diarmidmackenzie changed the title Updates to "Basic Scene" Guide Documentation: Guide update: Updates to "Basic Scene" Guide Jan 12, 2023
@diarmidmackenzie
Copy link
Contributor Author

Moved from examples\showcase to examples\docs.

@diarmidmackenzie
Copy link
Contributor Author

Following up on this PR & also #5211

I believe both can be merged. Unresolved issues that came up in review were:

auto-sync to glitch. Technically complicated. #5222 tracks the options here:
whether to refer to ../../../dist/aframe-master.js or aframe.io/releases/[release]/aframe.min.js - we agreed the best option is to refer to dist, with a comment indicating how to use aframe.io when copy/pasting the code. 5214 and 5215 already merged using this pattern. If we implement sync-to-glitch, it will be possible to adjust the code as part of the sync operation so that it works on glitch without a local aframe library.
#5211 (comment) - unfortunately it doesn't look like any good options available here. As per #5211 (comment), embedme is promising, but will create maintenance pain as code snippets have to be referenced by line number, rather than by tags.
While it would be nice to solve some of the above issues, none is simple to solve, and I don't see a good case for keeping outdated example code in the meantime.

So on the basis that we merged #5124 and #5215 already, I think we should be OK to merge this one & #5211

@dmarcos
Copy link
Member

dmarcos commented Nov 12, 2024

Ugh. sorry. this fell through the cracks and is a good contribution. I'm all about removing glitches from the docs.

If you can rebase I'll merge! Thanks so much

1st example shows environment component (initial part of tutorial)
2nd example shows custom environment (optional part of tutorial)
Since only the "custom environment" bit is marked as "optional" in the docs.
@diarmidmackenzie
Copy link
Contributor Author

Rebased.

I also reworked this a bit to deal with the "optional" part of the tutorial, so now we have 2 examples, one with the pre-canned environment, and one with the custom environment.

@dmarcos
Copy link
Member

dmarcos commented Nov 14, 2024

Thanks so much for jumping back on it so quickly

@dmarcos dmarcos merged commit 9cdc304 into aframevr:master Nov 14, 2024
3 checks passed
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.

3 participants