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

Fix Bug in Arbitrary Edit Tab and improvements in Tempo Widget. #2807

Merged
merged 3 commits into from
Feb 5, 2021

Conversation

daksh4469
Copy link
Member

@daksh4469 daksh4469 commented Feb 4, 2021

Issue Reference: #2767
In the Arbitrary Tab of the Add Pitches section in the Temperament Widget, I noticed that once if you hover on the outer circle before the animation is completed, it remains incomplete forever.

Before:

before.mp4

So, I just added a Timeout on the event listener of mouseover event so that it activates only after the animation is completed.

After:

after.mp4

Also, in the Tempo widget, it showed "Play" on the Pause button and vice-versa.

image

image

This was fixed in this PR.
Finally, there was no error shown on clicking the down button when the BPM is 30 or clicking the speed up button when the BPM is 1000.
Thus, I added an error message showing the subsequent errors:
For BPM=30 and clicking the down button:

image

For BPM=1000 and clicking the speed up button:

Screenshot from 2021-02-05 09-56-39

@daksh4469
Copy link
Member Author

Please review this PR @meganindya.
Thanks.

docById("wheelDiv3").addEventListener("mouseover", (e) => {
this.arbitraryEditSlider(e, angle1, ratios, pitchNumber);
});
},1500);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did you decide on this value: 1500ms?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Through trial and error.
I tried reducing 50ms from 2000ms and on the 1400ms mark, I felt it could show the "noteInfo" just before the whole animation could complete.
Thus, decided on the value of 1500ms.

@daksh4469 daksh4469 changed the title Fix Bug in Arbitrary Edit Tab Fix Bug in Arbitrary Edit Tab and improvements in Tempo Widget. Feb 5, 2021
@daksh4469
Copy link
Member Author

Is there any issue with this PR? @meganindya

@meganindya meganindya merged commit 35fe116 into sugarlabs:master Feb 5, 2021
@daksh4469 daksh4469 deleted the daksh4469-master branch February 5, 2021 14:46
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