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

Examples cleanup (2nd pass) #6041

Merged
merged 19 commits into from Feb 5, 2024
Merged

Examples cleanup (2nd pass) #6041

merged 19 commits into from Feb 5, 2024

Conversation

kpal81xd
Copy link
Contributor

@kpal81xd kpal81xd commented Feb 5, 2024

  • Removed unused files
  • Organises common files into folders
  • Moves files from src into app where possible
  • Removed unnecessary building of static data
  • Renamed scripts and added comments to be more informative
  • Extracted standalone html template into file for better readability
  • Checked thumbnails in (prevent regenerating when cloning)

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

@kpal81xd kpal81xd self-assigned this Feb 5, 2024
@kpal81xd kpal81xd requested a review from a team February 5, 2024 15:19
Copy link
Contributor

@willeastcott willeastcott left a comment

Choose a reason for hiding this comment

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

LGTM!

@willeastcott
Copy link
Contributor

I recommend updating this PR to include the thumbs in WebP format instead of PNG.

@kungfooman
Copy link
Collaborator

I use app.html and the importmap because it allows you to circumvent npm run ... commands - CTRL+R with DevTools open and your changes are reflected instantly (without any scripts running anywhere).

I like to have the screenshots in the repo and luckily the dist/thumbnails folder is only 8 MB right now.

We could probably also add the generated HTML files and make them work as-is (so devs can simply clone the repo into any web root and are able to hack away with only a npm install - and then the importmap does the dependency resolution into node_modules).

@kpal81xd
Copy link
Contributor Author

kpal81xd commented Feb 5, 2024

I use app.html and the importmap because it allows you to circumvent npm run ... commands - CTRL+R with DevTools open and your changes are reflected instantly (without any scripts running anywhere).

I like to have the screenshots in the repo and luckily the dist/thumbnails folder is only 8 MB right now.

We could probably also add the generated HTML files and make them work as-is (so devs can simply clone the repo into any web root and are able to hack away with only a npm install - and then the importmap does the dependency resolution into node_modules).

Ahh okay that makes sense. Yea im gonna redo the import map and get the examples generated properly instead of the current template system to allow for actual import statements

@kpal81xd kpal81xd merged commit 7b1477b into main Feb 5, 2024
7 checks passed
@kpal81xd kpal81xd deleted the examples-rework branch February 5, 2024 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants