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

added link to artist page #1182

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nickm980
Copy link

@nickm980 nickm980 commented Sep 6, 2023

Clicking on the artist name in the song player below the title of the song will navigate to the artist page. I believe this is consistent with most music player apps.

Ex) Spotify and the youtube music app have a feature where clicking on the artist name below the title of the page changes the page to the artist page.

The current way to access the artist page from ViMusic is to press the ellipses on the bottom right corner then "More of {artist}" button which requires an additional step. Instead of two steps, it should only take one. Adding the link to the artist name is a more intuitive approach and requires less friction, improving user experience because visiting the artist page is a common action.

demo

Clicking on the artist name in the song player below the title of the song will navigate to the artist page more similar to the player on youtube music
@th3y
Copy link

th3y commented Sep 7, 2023

You are taking the id from Database, if user has no items in database (First time), the app will crash.

You can use extras in player metadata to get the information:

var artistsInfo by remember { mutableStateOf(  binder.player.currentMediaItem?.mediaMetadata?.extras?.getStringArrayList("artistIds") ) }

Problem for me: it crash when there's more than one artist.

Update:
It was not because there's more than one artist. The artistIds was returning nothing in some songs (Displaying more than one artist), to fix this

 if (!artistsInfo.isNullOrEmpty()) {
    val goTo = artistRoute::global
    goTo(artistsInfo?.get(0))
} else {
    val goTo = albumRoute::global
    val albumId = binder.player.currentMediaItem?.mediaMetadata?.extras?.getString("albumId")
    if (!albumId.isNullOrBlank())
        goTo(albumId)
}

// ADD import it.vfsfitvnm.vimusic.ui.screens.albumRoute

As a workaround. Works than expected

@siggi1984
Copy link

hey bro vfsfitvnm, please allow your followers to further develop the app. to improve the user experience.

@owencz1998
Copy link

owencz1998 commented Sep 21, 2023

What about developing on harmony seems more active or innertune.
https://github.com/anandnet/Harmony-Music

https://github.com/z-huang/InnerTune

@vfsfitvnm
Copy link
Owner

This PR is not complete as it doesn't take into account that one track may have more artists, or there might be differences between "artists text" (the one showed on the screen, generally richer) and the "artist metadata" (the one showed in the menus).

I already attempted implementing this feature a year ago, and I eventually gave up as the hassle of handling edge cases wasn't worth a single tap save.

PS: Anyone is free to fork the project and continue the development there, so that any feature/code quality policy can be used 🙃

@Rayforest
Copy link

Is there any good fork of vimusic that is updated recently?

@siggi1984
Copy link

Is there any good fork of vimusic that is updated recently?

Take a look at the fork.

https://github.com/fast4x/RiMusic

@Rayforest
Copy link

Take a look at the fork.

https://github.com/fast4x/RiMusic

Thank you 🥰

@ServerMonk63
Copy link

Done

@sharunkumar
Copy link

Is there any good fork of vimusic that is updated recently?

Take a look at the fork.

https://github.com/fast4x/RiMusic

Someone needs to update this fork link in the README

@Harsha-070
Copy link

In my opinion the application was great and I was thinking of an Idea of making a widget for this application..

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.

None yet

9 participants