Add track information options for the NowPlaying view including composer, conductor, soloists

[schup011]

• Added a "Track Info" settings menu in the NowPlaying screen with three checkboxes:

  • Show composer: This adds an additional line above the track title that shows the composer with e.g. "Ludwig van Beethoven:"
  • Show conductor: This adds an additional line below the artist line that shows the conductor and the album line is used instead for displaying band/orchestra (conductor does not make sense without an orchestra)
  • Trackinfo for classical music: This adds descriptions to the artists like "Soloist: " or "Conductor: " and it removes empty artist lines, so it will not show "unknown artist", but remove the line from the layout.

• Added German and French translations for the new settings menu.

[schup011]

The last commit I added was not meant for this pull request. How can I remove this one from the pull request?

[kaaholst]

The last commit I added was not meant for this pull request. How can I remove this one from the pull request?

All commits you commit to your master branch and push to GitHub will be added to this pull request. So to make new commits without adding them to this pull request, create a new branch you can work on. I always use git from the command line. In this case, something like this git switch -c <new branch>. Then do your changes and commit them.

Removing the last commit from this pull request is worse because it has already been pushed. If you really want it, you have to do something like (on your master branch):

git reset -hard <id of the last commit you want to keep>
git push --force

Use git log (or git log --oneline) to find the commit you want to keep.

Notes

• Force pushing causes issues for those which have already pulled your repo. So I will wait until you're done.
• If you create a new branch before resetting master, you will keep your changes in the new branch.

[schup011]

Ok, did it. For the next time, I have definitely learned how not to push new commits to an already existing pull request. Thanks a lot for your help!!

[schup011]

I added an other commit, this time intended, with a small fix and addition of some comments.

[schup011]

@kaaholst Any more information you need about this topic?

[kaaholst]

@kaaholst Any more information you need about this topic?

I will be reviewing this now, I'll leave some comments if I find something. I would also like to do some limited testing, but I need some data for that. Can you give me some pointers? E.g. which tags I need for local music, and if there are any online music services with this information

[schup011]

OK, great. You need to fill the COMPOSER, BAND and CONDUCTOR tags. For LMS, it depends on which tagging format you use, the format that is used in mp3 files is somewhat limited, there is an option in LMS to treat one of these fields as BAND. In FLAC files, all these tags are available right ahead. I can prepare some files for you, but need some time. I do not know whether online music services use these tags. Normal online radio stations usually don't.

[kaaholst]

Please prepare some files, thanks you! I prefer flac format if possible

[schup011]

FLAC Audio Files.zip Here are the files. All of them have the COMPOSER tag filled, that is standard for classical music. What the other tags is concerned, there are different combinations in the files:

1: BAND, CONDUCTOR, ARTIST (multiple) 2: ARTIST (multiple) 3: BAND 10: BAND, CONDUCTOR, ARTIST (one) 11: ARTIST (one) 12: BAND, CONDUCTOR

There is a delimiter setting in LMS for the separation between multiple artists. You need to set that to comma ( "," ) in order that LMS recognizes multiple entries.

I tested these files and as far as I can see and judge, they are displayed well in NowPlaying with the new version.

Feel free to ask any questions.

[schup011]

Ok, do you want me to fix these issues by additional commits?

[schup011]

Will add further commits tomorrow.

[kaaholst]

Ok, do you want me to fix these issues by additional commits?

Yes, i think that is the easiest. But it's up to you, if you want to add new commits, or amend and force push

[schup011]

OK, I did all the changes you requested.

[schup011]

One more thing: Could we make those artist and album text lines also Marquee in order that they scroll automatically if the content is too long for the line? I tried to do that by using a similar style as that from the song title, but did not succeed and also the "RequestFocus" command did not help....

Could you give me a hint how this is done? Or are there reasons for not making those text lines Marquee?

[kaaholst]

One more thing: Could we make those artist and album text lines also Marquee in order that they scroll automatically if the content is too long for the line? I tried to do that by using a similar style as that from the song title, but did not succeed and also the "RequestFocus" command did not help....

Could you give me a hint how this is done? Or are there reasons for not making those text lines Marquee?

Try artistText.setSelected(true) in updateSongInfo or other relevant place. And also <item name="android:ellipsize">marquee</item>. This will make more elements Marquee at the same time, which may be confusing. Experiment a bit, and see what you think.

[kaaholst]

I have now tried the new functionality, and everything seems to work as described. Firstly, thanks for the contribution!

I have made a list of stuff we should consider.

• Defaults. What do we want for users which have not chosen the new preferences? This should preferably be what most users want.
• Instructions. While the new functionality works, it wasn't easy to figure out without the instructions you wrote in the this PR. Is there something we can do help the user? (it he wants to change the defaults)
• Why no soloist description if there is no band?
• Why the colon after the composer?
• Classical music info in now playing mini layout?
• Classical music info in album and track lists? (might be hard to achieve)
• It seems odd to check if the string contains a specific character “,”, to determine whether we have multiple artists. Instead, use the structured data we have in the Song object.

[schup011]

It seems odd to check if the string contains a specific character “,”, to determine whether we have multiple artists. Instead, use the structured data we have in the Song object.

Corrected that. Sorry, I missed that one before.

[schup011]

One more thing: Could we make those artist and album text lines also Marquee in order that they scroll automatically if the content is too long for the line? I tried to do that by using a similar style as that from the song title, but did not succeed and also the "RequestFocus" command did not help.... Could you give me a hint how this is done? Or are there reasons for not making those text lines Marquee?

Try artistText.setSelected(true) in updateSongInfo or other relevant place. And also <item name="android:ellipsize">marquee</item>. This will make more elements Marquee at the same time, which may be confusing. Experiment a bit, and see what you think.

I tested that and it works, thanks.

You are somewhat right that it can look strange. On the other hand, I would prefer to see all the information that is in the tag instead of seeing it just cut off. Anyway, in most cases artist information won't be so long.

So my suggestion is to go for it, and same for the album line.

[schup011]

Classical music info in now playing mini layout?

I have realized something like that (not pushed yet). Composer before the track title in the first line, and artist - band - conductor in the second line.

Two things to be considered there:

• Second line should be also marquee then. As with the marquees of the full height layout lines, this is rather an esthetic or cosmetic question. I personally prefer marquee.
• Now we come back to the settings question. I would not spend extra settings for the mini layout, but leaving it like now would mean that these are only available in the nowplaying screen, which is somehow weird. Solutions:
  • Make them again available also in jiveitemlist_menu?
  • Global preferences?

[schup011]

Defaults. What do we want for users which have not chosen the new preferences? This should preferably be what most users want.

I would leave all options unchecked as default. I am quite certain that this is what most users want, I am rather belonging to a minority.

Instructions. While the new functionality works, it wasn't easy to figure out without the instructions you wrote in the this PR. Is there something we can do help the user? (it he wants to change the defaults).

I could formulate some help comments, but you should say where to put them. Is it possible within the menu to provide a help link? Something like "About composer, band, conductor and classical"?

Why no soloist description if there is no band?

There is no right or false. My logic is that you only have to specify someone as soloist if there are also ensemble players or singers participating. If not, it is absolutely clear those single persons are soloists. But if you find that argument too special, we can remove that "logic" and could always display "Soloist: " or "Soloists: "

Why the colon after the composer?

Again, there is no right or false, it's a matter of taste. You can find both, with colon and without, maybe it is a little bit a German thing. We can remove it, if you want.

Classical music info in now playing mini layout?

See separate post.

Classical music info in album and track lists? (might be hard to achieve)

Well, I would not do it. The album and track lists follow a format that you can configure via LMS setting. I would leave that as it is.

It seems odd to check if the string contains a specific character “,”, to determine whether we have multiple artists. Instead, use the structured data we have in the Song object.

Done, see separate post and commit.

[kaaholst]

Classical music info in now playing mini layout?

* Second line should be also marquee then. As with the marquees of the full height layout lines, this is rather an esthetic or cosmetic question. I personally prefer marquee.

OK

* Now we come back to the settings question. I would not spend extra settings for the mini layout, but leaving it like now would mean that these are only available in the nowplaying screen, which is somehow weird. Solutions:
  * Make them again available also in  jiveitemlist_menu?
  * Global preferences?

I'm thinking global preferences (in new Now Playing section) But it's not a hard requirement, both are OK for me.

[schup011]

I'm thinking global preferences (in new Now Playing section) But it's not a hard requirement, both are OK for me.

Global preferences would have the benefit that much more explanation could be inserted. Maybe that is enough to guide users through those options. I will prepare something.

[kaaholst]

Defaults. What do we want for users which have not chosen the new preferences? This should preferably be what most users want.

I would leave all options unchecked as default. I am quite certain that this is what most users want, I am rather belonging to a minority.

I'm thinking, perhaps, most items will not have the classical music tags, so having the new settings on as default won't hurt. If it does, we have implemented possibility to keep the current behaviour. At any rate, the defaults is chosen in the methods in Preferences.java, but you probably already know this.

Instructions. While the new functionality works, it wasn't easy to figure out without the instructions you wrote in the this PR. Is there something we can do help the user? (it he wants to change the defaults).

I could formulate some help comments, but you should say where to put them. Is it possible within the menu to provide a help link? Something like "About composer, band, conductor and classical"?

Links are possible, I think also from a menu, see e.g. about_dialog.xml. You could add a page to the wiki and link to it.

Why no soloist description if there is no band?

There is no right or false. My logic is that you only have to specify someone as soloist if there are also ensemble players or singers participating. If not, it is absolutely clear those single persons are soloists. But if you find that argument too special, we can remove that "logic" and could always display "Soloist: " or "Soloists: "

Reason for asking is because the program logic would be simpler. It opens up more room for errors, and it also increases the tests to be performed with each release. But current solution is fine, if it gives a better uesder experience.

Why the colon after the composer?

Again, there is no right or false, it's a matter of taste. You can find both, with colon and without, maybe it is a little bit a German thing. We can remove it, if you want.

Maybe embed the colon into the localization texts. That way it can be customized for each language.

Classical music info in album and track lists? (might be hard to achieve)

Well, I would not do it. The album and track lists follow a format that you can configure via LMS setting. I would leave that as it is.

OK

[schup011]

Global preferences would have the benefit that much more explanation could be inserted. Maybe that is enough to guide users through those options. I will prepare something.

I have added the settings to global preferences (not pushed yet). Having it in both the NowPlaying trackinfo menu and the global preferences is not an option, is it? So should I then remove again the NowPlaying trackinfo menu?

[kaaholst]

I have added the settings to global preferences (not pushed yet). Having it in both the NowPlaying trackinfo menu and the global preferences is not an option, is it? So should I then remove again the NowPlaying trackinfo menu?

I'm fine with having the settings in both places. A bit like theme selection, which would otherwise only be possible from JiveItemLists.

[schup011]

I have added the settings to global preferences (not pushed yet). Having it in both the NowPlaying trackinfo menu and the global preferences is not an option, is it? So should I then remove again the NowPlaying trackinfo menu?

I'm fine with having the settings in both places. A bit like theme selection, which would otherwise only be possible from JiveItemLists.

Great. So I will push these and the other topics probably tonight, and then we are - hopefully - through.

[kaaholst]

Great. So I will push these and the other topics probably tonight, and then we are - hopefully - through.

One more thing 😊 GitHub claims that “This branch cannot be rebased due to conflicts”, so you need to merge or rebase from upstream. Let me know if you need any help with that.

[schup011]

Great. So I will push these and the other topics probably tonight, and then we are - hopefully - through.

One more thing 😊 GitHub claims that “This branch cannot be rebased due to conflicts”, so you need to merge or rebase from upstream. Let me know if you need any help with that.

Hm... I have not rebased, and now it shows "This branch has no conflicts with the base branch".

[schup011]

I'm thinking, perhaps, most items will not have the classical music tags, so having the new settings on as default won't hurt. If it does, we have implemented possibility to keep the current behaviour. At any rate, the defaults is chosen in the methods in Preferences.java, but you probably already know this.

No, I would leave definitely all defaults off, because in pop music the composer can be quite unknown, and it would be strange to quite a number of people to see that name. And most of them would rather see album instead of band...

Links are possible, I think also from a menu, see e.g. about_dialog.xml. You could add a page to the wiki and link to it.

I have not done that so far and now, it is in global settings, and quite well described (in my opinion).

Maybe embed the colon into the localization texts. That way it can be customized for each language.

I have removed the colon. Putting it into localization texts would cause other problems with the mini layout.

Mini layout

I have added the information into the two single lines.

So please check whether this is everything you mentioned / noticed.

[kaaholst]

Merged thanks for the contribution😊

[schup011]

Thanks a lot for your patience.

[schup011]

@kaaholst Can I use my branch for potential further pull requests or do I have to create a new one for that?

[kaaholst]

@kaaholst Can I use my branch for potential further pull requests or do I have to create a new one for that?

You can use your master branch or any other branch you wish, for your local development. You just need to create the pull request to the upstream development branch.

But you need to keep your local branch up to date with the upstream development branch. That might be easier, and less confusing, if you also have a development branch in your fork.

If you have a larger or experimental feature, it will be beneficial to create a feature branch.