ryohey / signal

Online MIDI Editor: signal
https://signal.vercel.app
MIT License
1.2k stars 136 forks source link

Play white keys when touched anywhere in its area #345

Closed abaresk closed 1 month ago

abaresk commented 1 month ago

This highlights and accepts touch inputs from the entire area of a white key, including the tabs underneath black keys:

Screenshot 2024-05-11 at 11 42 42 AM

vercel[bot] commented 1 month ago

@abaresk is attempting to deploy a commit to the Ryohei Kameyama's projects Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] commented 1 month ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
signal ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 13, 2024 2:36am
ryohey commented 1 month ago

@abaresk Very nice! It looks better than before. One small suggestion: you could simplify the code by adjusting the rendering logic to draw white keys first, then black keys. This change would allow you to remove the below code. https://github.com/ryohey/signal/blob/521809fe4945fba7d2ee45eada5c5117a0530e29/src/main/components/PianoRoll/PianoKeys.tsx#L60-L67

It looks like below:

for (let i = 0; i < numberOfKeys; i++) {
  const isWhite = ...
  if (isWhite) {
    drawWhiteKey(...)
  }
}

for (let i = 0; i < numberOfKeys; i++) {
  const isBlack = ...
  if (isBlack) {
    drawBlackKey(...)
  }
}
abaresk commented 1 month ago

I updated the rendering logic according to your suggestions. There still is some complexity as the white key height depends on its neighboring black keys. Let me know how this looks to you!

ryohey commented 1 month ago

Very clean code! Thanks.