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

Play white keys when touched anywhere in its area #345

Merged
merged 4 commits into from May 13, 2024

Conversation

abaresk
Copy link
Collaborator

@abaresk abaresk commented May 11, 2024

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

Copy link

vercel bot commented May 11, 2024

@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.

Copy link

vercel bot commented May 11, 2024

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
Copy link
Owner

ryohey commented May 11, 2024

@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.

if (prevKeyBlack) {
ctx.fillStyle = fillSytle
ctx.fillRect(blackKeyWidth, height / 2, width - blackKeyWidth, height)
}
if (nextKeyBlack) {
ctx.fillStyle = fillSytle
ctx.fillRect(blackKeyWidth, -height / 2, width - blackKeyWidth, height)
}

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
Copy link
Collaborator Author

abaresk commented May 12, 2024

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
Copy link
Owner

ryohey commented May 13, 2024

Very clean code! Thanks.

@ryohey ryohey merged commit ebcde51 into ryohey:main May 13, 2024
3 checks passed
@abaresk abaresk deleted the white-keys branch May 13, 2024 02:50
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.

None yet

2 participants