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 sign-images to tube descriptions #44

Merged
merged 2 commits into from
Oct 29, 2024

Conversation

mojoaxel
Copy link
Contributor

Hi folks!

This is my first pull-request. I'm a web-developer and member of a RoR Group in Erlangen, Germany and hope I can contribute more in the future.

Let me know what you think about my contribution and if you want me to change things.

See you at TNM24 !?

@tuxor1337
Copy link
Collaborator

tuxor1337 commented Oct 19, 2024

Thanks a lot for the PR, it's a great idea to add tune signs to the player!

Here are remarks about two signs in this PR:

  • Karla: The image file is called "kalashnikov", and the sign that is shown in the picture seems to be the one that used to be common when the tune was known under that name (finger pistol pointing to own head). As far as I know, for several years at least, the tune has been known under the name Karla or Karla Shnikov, and the tune sign has been changed to index and middle fingers raised, while other hand covers the other fingers. This is also the sign described (with words) in the karla-shnikov.md.
  • Samba-Reggae: The "smoking a cigar" sign was up-to-date at the time of this PR, but there was a decision at the TNM to replace it with an S formed with two hands.

If you don't have up-to-date pictures/images for those two, I suggest to remove them from this PR, and merge the others.

@mojoaxel
Copy link
Contributor Author

👍 Thanks for the feadback.

  • I have removed the image for "karla-shnikov" and "samba-reggae".
  • I also updated the sign text for "samba-reggae".

Let me know if I should do more improvements.

@tuxor1337 tuxor1337 merged commit d36cc7c into beatboxjs:main Oct 29, 2024
@cdauth
Copy link
Contributor

cdauth commented Jan 10, 2025

Sorry for taking forever to have a look at this.

First of all, thanks for the contribution. I think having the signs is really useful, and I also think that the tune description is a good place to display them.

I noticed a technical problem with the implementation: While the images show up fine in dev mode, in the final bundle they do not display. This seems to be related to hmsk/vite-plugin-markdown#346. I couldn’t find a quick solution for this (RoR Player is bundled as a single HTML file, so putting the images in the public folder and referencing them there is not a solution), so for now I have added the images as data URLs straight into the Markdown files. It would be good to find a better solution for this at some point.

I managed to half the size of the drawn signs by converting them to PNGs with an indexed colour palette (convert in.jpg -colors 256 out.png). SVGs would be perfect, but I couldn’t find a tool that would auto-trace these in acceptable quality. For the future it would be perfect if we could find someone to draw the signs as vector graphics, as that would probably require the least space if we add all the signs to the player.

I have made my changes on the main branch and will soon finally deploy this.

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