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

Change octaves in piano keyboard #3380

Merged
merged 8 commits into from
Jul 24, 2024

Conversation

ego-lay-atman-bay
Copy link
Contributor

I added the ability to change octaves in the piano keyboard.

image
image
image

I did also make sure to take into account the (computer) keyboard support.

I kept the ability to use the arrow keys to change notes (and it goes through all the octaves when the selected key reaches the edge of the keyboard). If you hold shift while pressing an arrow key, it changes the octave.

The letter key shortcuts are also usable (which selects the key in the current octave).

This addition is great, because it allows a selection of the full midi range, not just 2 octaves. It's better for users creating music, and not forcing the user to guess the correct midi number.

Adding a second octave was not nearly as hard as I thought it would be.
I added the third C key on the far right, as per jens request.

I also added a much more streamlined way of setting the number of visible octaves, so in case someone may want a keyboard with more, all you need to do is change a single variable (rather than a bunch of numbers in different functions).
Make sure the maximum octave follows the number of visible octaves
@jmoenig
Copy link
Owner

jmoenig commented Jul 24, 2024

Awesome, @ego-lay-atman-bay , thank you so much! And it comes just in time for v10 :)

[edit] I love this kind of PR that adds an almost invisible but madly useful feature.

@jmoenig jmoenig merged commit 72711df into jmoenig:master Jul 24, 2024
@jmoenig
Copy link
Owner

jmoenig commented Jul 24, 2024

BTW, If you send me a message or email with your real name I'll add an entry in the credits dialog and I'll also credit you in the sources. If you prefer to keep it private I'll mention your Github handle.

@ego-lay-atman-bay
Copy link
Contributor Author

I prefer to keep my real name private, so my username would be prefered.

@jmoenig
Copy link
Owner

jmoenig commented Jul 24, 2024

Excellent, will do. Thank you, great job!

@jmoenig
Copy link
Owner

jmoenig commented Jul 24, 2024

I've integrated your pull request, and tweaked a few minor details: 370a359c3aa2786b8c87bf903d83dd0ddfb06d5c

Just some little stuff JSHint complained about. Also the direction when holding down the left/right arrow keys seemed to be reversed, and two methods apparently didn't ever get called, so I took them out.

If you get a chance to look over these tweaks please let me know if I misunderstood anything. I'll create another release candidate now, later today I'll release v10 with whatever we have.

Again, thank you!

@jmoenig
Copy link
Owner

jmoenig commented Jul 24, 2024

it's now also online in rc6, if you'd like to give it a try

@ego-lay-atman-bay
Copy link
Contributor Author

ego-lay-atman-bay commented Jul 24, 2024

The 2 methods you removed (selextUp and selectDown) were used, until you replaced

if (event.shiftKey) {
    return this.octaveUp();
} else {
    return this.selectUp();
}

with

return event.shiftKey ?
this.octaveDown()
: this.octaveUp();

(same with the down arrow key)

This results in changing the octave whenever you press the arrow key, and you can't change the note with the arrow keys (that's what the selectUp and selectDown functions are used for).

The arrow keys were not reversed, I know for certain, because I tested then out many many times.

If you press the left/down arrow key, the note goes down. If you hold shift, the octave goes down.
If you press the right/up arrow key, the note goes up. If you hold shift, the octave goes up.

@jmoenig
Copy link
Owner

jmoenig commented Jul 24, 2024

Ah, so is the current behavior now wrong?

@ego-lay-atman-bay
Copy link
Contributor Author

Yes, it now behaves like this.

If you press the left/down key, the octave goes down. If you hold shift, the octave goes up.
If you press the right/up arrow key, the octave goes up. If you hold shift, the octave goes down.

@jmoenig
Copy link
Owner

jmoenig commented Jul 24, 2024

Yep, that's what I thought should happen. What did you want to happen?

@ego-lay-atman-bay
Copy link
Contributor Author

If you press the left/down arrow key, the note goes down. If you hold shift, the octave goes down.
If you press the right/up arrow key, the note goes up. If you hold shift, the octave goes up.

The note changing was a feature of the old piano keyboard, which I want to keep, that's why I made octave switching be done by holding shift. It's much more intuitive for people using the arrow keys to move notes, and not have to rely on pressing the right letter key.

@jmoenig
Copy link
Owner

jmoenig commented Jul 24, 2024

Ah, okay, so let me revert those changes ... in a minute (I'm busy with something else right now). Thanks!

@jmoenig
Copy link
Owner

jmoenig commented Jul 24, 2024

I've reverted to your original behavior: 99a4890
and made another release candidate. If you find the time can you confirm that this is now how you intended it to work?
Thanks!

@ego-lay-atman-bay
Copy link
Contributor Author

That looks right, thanks!

@jmoenig
Copy link
Owner

jmoenig commented Jul 24, 2024

Thank you for the quick test & confirmation, and for this beautiful feature!

@ego-lay-atman-bay ego-lay-atman-bay deleted the piano-keyboard-octaves branch December 5, 2024 18:28
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