search

mattttvaughn / chronicle

GNU General Public License v3.0
216 stars 58 forks source link

New speed chooser ui #66

Closed lks-nbg closed 2 years ago

lks-nbg commented 2 years ago

This PR is a complete redesign of the current speed chooser. Instead of selecting options from a given list the user can now choose a custom playback speed between 0.5 and 3.0 in 0.05 steps with a slider.

Screenshot_20220330-175956_2

During testing I received some leak notifications, but I am not sure if this was caused by this PR. It should definitely be thoroughly checked and tested. Furthermore I noticed that sometimes the slider value is stuck and does not get updated. The actual playback speed and the button in the player however are working as expected.

One thing to mention is that the current implementation relies on materialLibVersion = '1.6.0-beta01' which allows to show the slider label all the time and not only when touched (app:labelBehavior="visible"). If this dependency is not accepted, we can change that and start with the progress only visible while sliding and change it as soon as 1.6.0 is published as stable.

Resolves #7 / Alternative to #63

mattttvaughn commented 2 years ago

The new leak notifications have been around for a few days fyi, related to a toast somewhere if i recall but haven't looked into it yet

lks-nbg commented 2 years ago

Thanks for reviewing. I made the suggested changes.

But I am still having trouble updating the change_speed_button. I added observers for speed & playbackSpeedString for debugging and noticed that these variables do not change after updatePlaybackSpeed is called. Any ideas on how to solve this?

mattttvaughn commented 2 years ago

Pushed a commit which should resolve a few issues. Apologies for not letting you working things out but I ended up making most of the changes while debugging the issue and figured better to not let it go to waste.

Will add some comments to the PR explaining the issues they resolve

Feel free to review it if you see any issues

lks-nbg commented 2 years ago

Pushed a commit which should resolve a few issues. Apologies for not letting you working things out but I ended up making most of the changes while debugging the issue and figured better to not let it go to waste.

No worries. I reached my limits and was not able to finish it myself. So I am grateful and happy to learn from your experience. The code looks good to me and during testing everything worked as expected 👍 From my side, this can be merged to get user feedback.