search

jessuni / shikwasa

An audio player born for podcast
https://shikwasa.js.org
MIT License
476 stars 30 forks source link

Fix: playbackRate #67

Closed yenche123 closed 2 years ago

yenche123 commented 2 years ago

Please refer to the commits

jessuni commented 2 years ago

I fixed a lint error that fails the test. As you refactored the mute feature, I thought we might as well remove all logic concerning attributes to UI and control them from there.

yenche123 commented 2 years ago

@jessuni

I fixed a lint error that fails the test. As you refactored the mute feature, I thought we might as well remove all logic concerning attributes to UI and control them from there.

I don't think use toggleAttribute is a good idea. If a developer writes player.muted = true but actually the muted property on player has been true, then the operation from the developer will not work.

jessuni commented 2 years ago

You might need to look twice – toggleAttribute actually checks before setting the attribute value. The term 'toggle' here means the function targets boolean attributes specifically, not simply switching between true and false. Thanks for the heads up though.