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

Pitch slider #2493

Merged
merged 7 commits into from
Aug 18, 2020
Merged

Pitch slider #2493

merged 7 commits into from
Aug 18, 2020

Conversation

sksum
Copy link
Member

@sksum sksum commented Aug 12, 2020

#2485 (comment)
@pikurasa I have added a continous tone for the slider

another suggestion:
image
rather than using this custom slider we could use =>
image

@pikurasa
Copy link
Collaborator

Right now there is no way of generating the pitch.

Upon cursor click, a hertz block must be generated.

@sksum sksum marked this pull request as ready for review August 13, 2020 13:38
@pikurasa
Copy link
Collaborator

@sksum

This is better.

A couple things:

  1. In music, ratios are important. The current boundaries of 240 and 700 are too arbitrary. Rather, we should have the max be 2 times the input pitch and min be half the input pitch. So for 392, the higher (max) should be 392*2=784 and the lower (min) should be 392/2 = 196. Essentially, this gives us an octave above and an octave below

(Also, right now, if you put a number above 700, you cannot slide around the pitch whatsoever.)

  1. Please use a wider width for the slider. It is not utilizing its real estate very well.

  2. The tick up and down calculation, I believe, was based on the chromatic scale. The math is something like x^(y/12) (Please verify). I wouldn't mind a calculation based on ratios either (like "just" intonation). However, adding/subtracting increments of 50 is not helpful.

  3. I am unsure about click and release, but defer to Walter on this aspect of the design.

@walterbender
Copy link
Member

Do we really want it to close every time you make a note?

@sksum
Copy link
Member Author

sksum commented Aug 14, 2020

made suggested changes.

@walterbender
Copy link
Member

There is still something peculiar with the UX. If I use the up/down arrows, I have no way to select a pitch.
Also, please test with multiple hertz blocks. That doesn't seem to work any more.

@sksum
Copy link
Member Author

sksum commented Aug 17, 2020

multiple hertz blocks and the save button should work now

@pikurasa
Copy link
Collaborator

This works much better.

I wish that the slider were wider--using the full width (the area where the blue is).

@sksum
Copy link
Member Author

sksum commented Aug 17, 2020

This works much better.

I wish that the slider were wider--using the full width (the area where the blue is).

what browser are you testing with ?
also could you please upload an image

@pikurasa
Copy link
Collaborator

Chromium

Screenshot at 2020-08-17 08:32:53

I imagine it would be nice to utilize the full width, not just the narrow green line.

Maybe instead of a circle for the slider point, have a horizontal bar.

change slider thumb style for the pitch slider widget
@pikurasa
Copy link
Collaborator

I like the new design a lot better.

I think the length of the note when you tick up and down should be longer (a quarter note). Right now, the pitch is so short it is hard to hear.

Other than that, I think we are good!

@pikurasa
Copy link
Collaborator

Yes, perfect length. Thanks!

@walterbender walterbender merged commit 84bbd96 into sugarlabs:master Aug 18, 2020
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.

3 participants