[FR] #341

[SurFace81]

#341 - Scroll minimized player to change song

[nift4]

Hi, thanks for the PR. I very much appreciate you going through these backlog items I never have time for.
Can you try to rebase the PR instead of merging back beta branch? It got a bit messy because I force pushed to fix a mistake in mailmap (sorry about that).
Also while the fix for #619 is a no-brainer to merge (I've cherry picked it just now to simplify rebasing), I'm not yet sure about swipe to change track in the mini player. Both in general (how I want to react to motion in the app, if I agree with the opinions in the feature request, whether it should be a toggle, whether it should hide normal next button) and specifically about the fact there isn't an animation, but good motion design generally includes animation. Also I didn't look at the code in detail yet but usually for consistent swipe UX in Android you use GestureDetector to detect swipes. Also now that weekend is almost over here I'm gonna be a bit short on time again so I'll need some time to think about swipe on miniplayer in detail and maybe consult with duo.

[SurFace81]

So I should create a new branch for these changes and close this PR, right?

[nift4]

a PR is always synchronized to a branch while it's open. You can also keep using the same branch by overwriting it with a force push.
if you force push the branch, the PR will be updated. so it's fine to keep PR open (the outdated title/description doesn't matter, and even if it does, it can be edited).

In general you would run:

git fetch https://github.com/FoedusProgramme/Gramophone beta
git reset --hard FETCH_HEAD
git cherry-pick 8106919a160c1774af7a1d78b5f118a476fc411d
git push -f <your remote name / github url> beta

to do a destructive resynchronization with my branch, and then take the commit you made which isn't merged yet.

[SurFace81]

Looks like everything’s correct now.

[nift4]

Hi, thanks again for the PR.

so after thinking it over, I would like there to be an animation that reacts to the drag before this is merged. That is important for discoverability (so people know why the song just changed) and it also just looks better.
Something I have in mind is that dragging/flinging the mini player from the sides makes it slide in drag direction, and where would be empty space is a copy of it slide in from the side, which already contains the song that will be played if the drag is completed.
It's also better to not hide the next button, and maybe don't even offer a toggle for the feature. Instead, it should just be tuned to be hard to trigger accidentally. While I don't 100% agree with the essay, I think https://ometer.com/preferences.html is a good explainer on why less preferences is generally a good thing.

Additionally, please try to use GestureDetector instead of manual MotionEvent handling, it is less error prone.

(If you ping me in an issue before starting to work on it, I could also outline possible considerations before you start, if you want to.)

[SurFace81]

Hi, I’ll do that.

[nift4]

I see you've already noticed that Panda sniped you with his PR, he told me he didn't see yours yesterday. My sincere apologies for that.

[SurFace81]

Shall I share my solution for reference?

[nift4]

I'm curious how differently you did it, so that would be nice