-
Notifications
You must be signed in to change notification settings - Fork 934
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
Fix: #4128 White and Black key generation is consistent #4129
Fix: #4128 White and Black key generation is consistent #4129
Conversation
It took me a minute to find the changes related to the problem you set out to fix as there are multiple formatting changes mixed in. Please put the formatting changes should be in a separate commit in the future. How does this change work? Why is only necessary to add a second octave worth of black note positions? Why not more? Should we calculate this array instead of hard-code it? |
Yes sir, I will keep this in mind and won't do it next time. I will disable my formatting extension. In case of octaves, on inspecting the code what I have understood is that there is a limit up to which the keys can be generated. I believe the last note name is C9. The createKeyboard function accounts a seperate array for black notes. This particular array (where I have made the changes) holds the excluded black notes). That's how it is able to know which black note to render. Of course I also thought about making a function which will make a list of excluded black notes but that would be unnecessary I suppose. |
Just keep the formatting in a separate commit. Meanwhile, I'll try to better understand what is going on here. |
Okay I unerstood now. But it is still limited so I suppose we can just estend the 'excluded black notes' array. What do you think sir ? If you agree then I will make the necessary changes. |
Extending the array is fine. |
@walterbender Sir the fix is here. Also their is no formating. |
Please make a separate PR with the formatting update. |
Is there any standard formatting style for music blocks, that I can follow? |
Issue #4128
With this PR, the music keyboard will be able to add consistent white and black keys throughout its addition until the end of pitch. Previously, after a certain number of addition of Pitch notes the black notes started to break there sequence.
@walterbender Sir, kindly check this PR and give a feedback if this is relevant or not. Thank you.