fix #454 overflowing of song title

[DOD-101]

This PR needs to be merged after #482 resolved

Fix #454 by splitting the metadata text and the "title" text into two
different rects. This means that the title text being cut off to long
won't cause the metadata text to overflow outside the viewport.

This does however mean that the playback_format config option should
no longer contain the {metadata} placeholder.

[aome510]

This PR needs to be merged after #482

Why is this the case? I thought you can create an upstream PR from a branch in your fork. Separate two PRs gonna make reviewing easier unless two PRs are strictly dependent

[aome510]

I thought the overflow can be fixed by increasing playback_window_width. I don't see why we need a separate title_height?

[apprehensions]

The title should simply be truncated if it can't fit fully. The official spotify client 'wraps' the title by moving it from left to right, but i don't believe that will be possible here.

[apprehensions]

Could you squash your commits?

[aome510]

The title should simply be truncated if it can't fit fully. The official spotify client 'wraps' the title by moving it from left to right, but i don't believe that will be possible here.

truncating can be done by disabling wrapping when rendering the playback info.

spotify-player/spotify_player/src/ui/playback.rs

Line 105 in 2030b4e

let playback_desc = Paragraph::new(playback_text).wrap(Wrap { trim: false });

[DOD-101]

truncating can be done by disabling wrapping when rendering the playback info.

Yes, however this would mean also cutting off the metadata info and what's if some people would rather have the text wrap so they can actually read it.

I thought the overflow can be fixed by increasing playback_window_width. I don't see why we need a separate title_height?

The problem with this is that some people have their metadata at the bottom of the playback window. When the text now wraps the metadata disappears.

vs

Adding a separate rect for the metadata would fix this. It would mean that the title / album / artist wrapping wouldn't affect the position of the metadata.

The title should simply be truncated if it can't fit fully.

This can be done by simply setting title_height to 1 (which is the default).

[DOD-101]

Could you squash your commits?

Yes, but why?

Since the commits get squashed when merged into main it makes seems to me there is no difference once the PR is merged.

Why would you like me to squash them?

[aome510]

The problem with this is that some people have their metadata at the bottom of the playback window. When the text now wraps the metadata disappears.

I don't see how this can happen. The whole playback info is constructed as a single paragraph widget. Wrapping in one line shouldn't affect others. See the below picture, title line is wrapped but it doesn't affect other lines.

@apprehensions' problem in the original issue happened because of the wrapping as he used spaces to represent paddings between playback section and cover image section, which doesn't work with text wrap. My suggestion is just simply disabling wrapping, then no overflowing issue will happen.

However, I can see that text wrapping can be convenient feature for some people.

[DOD-101]

Wrapping in one line shouldn't affect others.

I'm not entirely sure what you mean by "affect", but what happens to me is that when the title line wraps it now takes up two lines, which means the line for the metadata is now no longer visible, because it is pushed down a line.

Here is my config for reference:

playback_format = """
{track} • {album} • {artists}



{metadata}"""
playback_window_height = 6

[aome510]

Wrapping in one line shouldn't affect others.

I'm not entirely sure what you mean by "affect", but what happens to me is that when the title line wraps it now takes up two lines, which means the line for the metadata is now no longer visible, because it is pushed down a line.

Here is my config for reference:

playback_format = """
{track} • {album} • {artists}



{metadata}"""
playback_window_height = 6

Can you try replacing whitespace with newline (\n)? Maybe whitespace doesn’t work well with text wrapping

[DOD-101]

Can you try replacing whitespace with newline (\n)? Maybe whitespace doesn’t work well with text wrapping

Same thing happens.

This is the new config.

playback_format = """{track} • {album} • {artists}\n\n\n\n{metadata}"""

[aome510]

Can you try replacing whitespace with newline (\n)? Maybe whitespace doesn’t work well with text wrapping

Same thing happens.

This is the new config.

playback_format = """{track} • {album} • {artists}\n\n\n\n{metadata}"""

Ah I see, it's because you add extra lines to push the metadata to the end. Therefore, it's expected for the metadata part to be moved outside the view upon overflowing.

[aome510]

Let's disable wrapping then. It's much simpler and will also resolve your problem.

[DOD-101]

Ah I see, it's because you add extra lines to push the metadata to the end.

Yup.

Let's disable wrapping then. It's much simpler and will also resolve your problem.

I can implement this if you like, but since we can achieve the same with setting the title_height to 1 as a default and give people the option to adjust it using the option. (Effectively toggling wrapping) It is the option I prefer / suggest.

[aome510]

I don't like separating metadata and other sections because it adds extra complexity while being less flexible, e.g. what if user wants to render metadata before album, artist?

[DOD-101]

Fair point. I'll go ahead and change the code to turn off wrapping.

[apprehensions]

While the title and album look like theyre simply truncated, there is no visual indicator such as ..., additionally, the album cover is missing.

[aome510]

While the title and album look like theyre simply truncated, there is no visual indicator such as ..., additionally, the album cover is missing.

probably because it failed to save the image file when album's name is too long