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

Collaboration to issue #534 #648

Merged
merged 2 commits into from
Apr 19, 2024

Conversation

wilckerson
Copy link
Contributor

@wilckerson wilckerson commented Apr 15, 2024

Hello everyone.

I have been using the Scale Workshop since V1 and I would like to congratulate you all for the amazing work and the new features added to each version.

I am a software developer and microtonal enthusiast as well, developing my own tools to explore this new musical world, like:
https://microtonal-keyboard.onrender.com
https://microtonal-tuner.onrender.com

However, I would like to start collaborating more with the existing tools in the community.

This PR is my contribution to handling the issue #534 that I have been missing for a while.

I have adjusted the code to display the label, ratio, cents, and frequency on the VirtualKeyboardKey considering the light/dark theme and the automatic colors. In a future PR, I am thinking of adding a left panel to the VirtualKeyboard page to allow the user to configure which of those info to display, and other related configurations, like the isomorphic vertical and horizontal values.

image
image
image

I hope this can be useful because I am excited to collaborate more with the project. Also, it would be awesome if this could be included in the V2 as well.

Thank you.

@frostburn
Copy link
Member

Screenshots look great! Will review after breakfast.

I hope this can be useful because I am excited to collaborate more with the project. Also, it would be awesome if this could be included in the V2 as well.

We're now in the beta cycle for version 3, so if you want to backport it to current line of releases on sevish and plainsound, branch off of the two branch and make another PR targetting it:
https://github.com/xenharmonic-devs/scale-workshop/tree/two

@frostburn
Copy link
Member

If you're interested in further collaboration I recommend joining our discord channel. You should find an invitation link on the About page.

@frostburn
Copy link
Member

The labels are sometimes offset by 1. We should pass in scale.colorForIndex as colorMap and scale.labelForIndex as labelMap similar to how the VirtualPiano component works.

@frostburn
Copy link
Member

I wonder if there's a CSS trick that's color-aware in order to switch text color based on context. Now it's hard to read the labels on pink notes.

@frostburn
Copy link
Member

This behavior should be toggleable using a setting. Makes sense to have it on by default, but the text gets busy on a mobile screen and some people will miss the old look.

@frostburn
Copy link
Member

Remember to add your name on the About page if you wish to be credited.

@wilckerson
Copy link
Contributor Author

Thanks for the feedback. I will address them. 👍

@wilckerson
Copy link
Contributor Author

@frostburn I have handled your comments, except the one about the colors which I don't have a better solution for now.
Also, I have included the setting to display the keyboard notes information (The default value is TRUE).
image

@frostburn
Copy link
Member

Nice work! Good to go once you remove the unused prop and squash the commits into one.

@frostburn frostburn linked an issue Apr 17, 2024 that may be closed by this pull request
@frostburn
Copy link
Member

Missing a Wilckerson - <i>developer</i> <br /> line on the about page, (unless you don't want to be credited).

author Wilckerson <[email protected]> 1713492886 -0700
committer Wilckerson <[email protected]> 1713493197 -0700

Displaying labels on Virtual keyboard
@wilckerson wilckerson force-pushed the virtual-keyboard-labels branch from 754ed9a to 59316de Compare April 19, 2024 02:31
@wilckerson
Copy link
Contributor Author

@frostburn I have squashed my commits. Thanks for this feedback.

@frostburn
Copy link
Member

Looking good. I'll review and do a minor squash tweak to the commit message to ref the issue.

:noteOn="noteOn"
:heldNotes="state.heldNotes"
:baseFrequency="scale.baseFrequency"
Copy link
Member

Choose a reason for hiding this comment

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

There's been a beta API change and this store prop no longer exists. I'll still merge your PR and fix on main. Normal beta cycle churn, nothing to worry about.

@frostburn frostburn merged commit cd800e3 into xenharmonic-devs:main Apr 19, 2024
2 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.

SW3: Add note labels to virtual keyboard
2 participants