-
Notifications
You must be signed in to change notification settings - Fork 124
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add volume controller #58
Conversation
Closes #56
Nice PR, but I would like to see us use and skin the native range input element, that will allow us to read the value from the native element. |
@MichielDeMey fair point, I absolutely forgot that |
} | ||
|
||
this.setState({ | ||
lastVolume: volume, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make sure we have a fixed length.
Using volume.toFixed(2)
will truncate the volume level to 2 numbers at the most.
e.g. 0.45555555
will become 0.45
Let's also persist the volume level (but not the "muted" state) to localStorage when the user closes the application, I believe that would improve UX a bit. |
Set volume to 100% when it has been set to 0 manually and user clicks toggle mute button rather than setting to previous value (which is a volume change step) Also, make volume value to be fixed-length when changing it using mouse wheel
@MichielDeMey just pushed changes that implement all your suggestions 馃憤 |
Just wondering if anything prevents this PR from being merged? |
Whoops, we totally lost track of this issue. Thank you for your contribution! |
Just came here looking for volume control support. Thanks for doing this! 馃檶 |
Released as v0.8.0 |
Thanks for all the hard work!! |
Volume controller that handles both clicks and wheel (trackpad) movement when mouse is hovering the controller 馃槑
Closes #56