Lyric support

[SO9010]

This is my start-to-finish feature request #349; I believe I have done it in a similar way to LebreSpot.

Currently what I have done is implement the API request and a very rough UI for it. Many improvements are needed for the UI, but currently, there is a new button at the bottom left that redirects you to a page called lyrics when you click it.

I want to implement time-synced lyrics, which shouldn't be too hard.

I would say its about 50% complete.

[jacksongoode]

This looks great, I would also think we should have a right click to lyrics too? Maybe that's too much? I'm also fine with not having the time syncing with the lyrics which I think might be difficult to render properly and cause CPU cycles?

[SO9010]

I think having it just as a toggle button makes more sense, but if you think we should have a right click option I can also add that?

[SO9010]

Do you think we should have a skip to section for when someone clicks on the line, the API includes a time stamp for each line.

[jacksongoode]

Yeah that could be a nice alternative to having the line highlighted and listening for the playback location. We could just click and seek to the location defined by the lyric at that time. We would need to make sure that the lyric displayed is always connected with the current song being played. Otherwise you might see a lyrics screen for another song? I think that could be okay, we just need to make sure that clicking on the lines will only seek if the song being played is that same song.

[jacksongoode]

Looks close!

• Clicking on the line to select the timestamp it seems to work well
• The text seems a little bit small maybe we could try with text medium rather than text small
• It would be nice to have some title of the song along with the artist and album at the top of the page so there's at least some indication as to where the lyrics are coming from

Other than that, I think it's ready to go.

[SO9010]

@jacksongoode I could do a similar thing to what we have ATM with the album view.

[jacksongoode]

I added caching and better formatting with the titles so I think this is all ready for review (just have a look at the last two commits and test it out). We still need a better lyrics icon I think...

[SO9010]

Hello, I undid your changes to the client file as you removed the deserialization part, which made it impossible to get any songs. Also, I'm pretty sure the way you cached does essentially the same thing as the load_cached function. Do correct me if I'm wrong.

Other than that I think this is looking very good, I just need to change the lyrics icon (that was just a placeholder). Then it should be good to get pushed to main!

Also thank you very much :)

[jacksongoode]

I just reverted the methods that we didn't need.

One nice thing I think would be if the user clicked on the lyrics button, it would actually toggle the lyrics as opposed to showing them. This could be introduced here if you feel like you have a good sense of how to do it, or we can add it in another PR. It just seemed redundant to have the lyrics button do nothing if you're already on the lyrics page. It feels intuitive that it should be a toggle that would go back to the previous state in the history.

[SO9010]

Ok I will have a look into this tomorrow!

[SO9010]

I can't quite figure out how to do it. I need to have AppState as a lens for the button to see the history of navigation, and I can't figure out how to implement that without causing many errors or rewriting a lot of the code.

So maybe it's a good idea to open up a separate issue for it.

[jacksongoode]

@SO9010 I just figured out a simple way of doing it. I'll merge once the builds work.