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

refractor/pitchslider:fixed linting errors and added js docs #2791

Merged
merged 2 commits into from
Jan 31, 2021

Conversation

abhishekkumar08
Copy link
Contributor

Refrences #2767, #2630, #2609

@abhishekkumar08 abhishekkumar08 changed the title fixed linting errors and added js docs refractor/pitchslider:fixed linting errors and added js docs Jan 29, 2021
@abhishekkumar08
Copy link
Contributor Author

@meganindya Have a look.
'_' is assigned a value but never used.
This error is in a loop how to fix this.?

@meganindya
Copy link
Member

for (let i = 0; i < this.frequencies.length; i++)

in this case

@abhishekkumar08 abhishekkumar08 marked this pull request as ready for review January 29, 2021 18:56
@abhishekkumar08
Copy link
Contributor Author

@meganindya please review this.Thanks

Copy link
Member

@meganindya meganindya left a comment

Choose a reason for hiding this comment

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

This is fine. But, it would be nice to have a feature. When you're sliding, the value with the hertz block in the widget block doesn't actually change. I think that'd behavior would be better. Please try if you can add that. Shouldn't be difficult.

@abhishekkumar08
Copy link
Contributor Author

@meganindya I used the frequencey[id] value but coudn't get the desired results, can you tell how should I do this?

@meganindya
Copy link
Member

Honestly, I have no idea. Need to look. Anyway, that could be another PR. I'm merging this.

@meganindya meganindya merged commit 47b47f9 into sugarlabs:master Jan 31, 2021
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