Skip to content
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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add jump to track command #501

Merged
merged 2 commits into from
Jul 12, 2024
Merged

Conversation

aNNiMON
Copy link
Contributor

@aNNiMON aNNiMON commented Jul 10, 2024

Resolves #484

I decided to refresh my ancient Rust knowledge and resolve my own feature request by myself 😄

It turns out that the simple jump to track action is better suited for this feature than the alternative of pressing # and entering a track number. The latter requires a track position to be persistent in the model, while currently track numbers are calculated during rendering.

@aNNiMON aNNiMON force-pushed the action-jumptotrack branch from 0d79d0f to 593b7ba Compare July 10, 2024 18:58
@aome510
Copy link
Owner

aome510 commented Jul 11, 2024

Thanks for updating @aNNiMON. As I think about this more, I think it's better to define the feature as a Command instead of a Action. The reason is that this feature is specific to the currently playing track while a track action should be generic and self-explanatory. For example, it has no use when you call that action in an already selected track in the context.

Then the command can be named JumpToCurrentTrackInContext or something with a detailed description so that user can know how to use it.

@aNNiMON aNNiMON force-pushed the action-jumptotrack branch from 593b7ba to e9827d2 Compare July 12, 2024 08:57
@aNNiMON aNNiMON changed the title add jump to track action add jump to track command Jul 12, 2024
@aNNiMON
Copy link
Contributor Author

aNNiMON commented Jul 12, 2024

@aome510 done. I'm using g c key map here (c — current track). Let me know if this key map should be changed to something else (e.g. g 0).

@aome510
Copy link
Owner

aome510 commented Jul 12, 2024

@aome510 done. I'm using g c key map here (c — current track). Let me know if this key map should be changed to something else (e.g. g 0).

g c makes sense. I pushed a change myself to make the handling logic more clear. It also now supports all the context pages instead of just the Tracks page. The nested callbacks are quite hard to follow

@aome510 aome510 merged commit ed9c4d4 into aome510:master Jul 12, 2024
3 checks passed
juliamertz pushed a commit to juliamertz/spotify-player that referenced this pull request Jul 20, 2024
Co-authored-by: Thang Pham <phamducthang1234@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jump to currently playing track in the list
2 participants