mattttvaughn / chronicle

GNU General Public License v3.0
219 stars 60 forks source link

Show tooltip while sliding progress bar #55

Closed lks-nbg closed 2 years ago

lks-nbg commented 2 years ago

I replaced SeekBar with Material Slider, which comes with an integrated value label. Resolves #37

There is one small issue that already occurs with the current implementation of SeekBar. The progress is continuously updated and if the slider is touched but not moving it sometimes jumps to the new position and returns after moving again. With onStartTrackingTouch & onStopTrackingTouch I could add a variable like "isSliding" but I don't know where it can be used to prevent updating the progress, especially when the chapter_progress TextView should continue to be updated.

mattttvaughn commented 2 years ago

Ah I see what you mean.

Not a big worry imo since it was already in place, if you don't have a solution ready to go you don't need to solve that in this PR.

But if you do want to solve it, here's an idea. Capture that isSliding value in the viewmodel like you were talking about, and use it to prevent emitting from chapterProgress/trackProgress like so (note: boolean might be flipped):

    val chapterProgress = currentlyPlaying.chapter.combine(currentlyPlaying.track)
    { chapter: Chapter, track: MediaItemTrack ->
        track.progress - chapter.startTimeOffset
    }.filter { !isScrubbing }
        .asLiveData(viewModelScope.coroutineContext)

    val trackProgress = currentlyPlaying.track
        .filter { !isScrubbing }
        .map { it.progress }
            .asLiveData(viewModelScope.coroutineContext)

And for chapter_progress, two ways to approach: either emit a different progress that you don't filter, or listen to the change listener for the seekbar, and then when isSliding is true, show the value currently being seeked to as the value of chapter_progress

Also, not sure how familiar you are with Flow/LiveData, but feel free to ask Qs about implementation if you have any

P.S. PR as it stands looks good to me, I don't see any issues in it. Up to you how you want to proceed

lks-nbg commented 2 years ago

I created extra variables with your filter solution and it works as expected.