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 Marquee to Make Long Text Readable #767

Merged
merged 12 commits into from
Dec 26, 2024

Conversation

maximmaxim345
Copy link
Contributor

@maximmaxim345 maximmaxim345 commented Dec 2, 2024

Current Situation

Currently, Music Assistant on mobile devices cuts off long titles, albums, and artist names, making it impossible to read them completely.
This is especially problematic with Classical Music, where titles often include the composer, composition name, and movement.

Overview

To address this issue, this PR adds a MarqueeText component, which automatically scrolls text that doesn't fit on screen.

To improve readability, MarqueeText includes the following features for use in Music Assistant:

  • Constant Scrolling Speed: Text scrolls at a consistent speed, regardless of length
  • Synchronization: Multiple instances of MarqueeText can be synchronized with MarqueeTextSync for coordinated scrolling

MarqueeTextSync

When a MarqueeTextSync object is passed through the sync prop of MarqueeText, the reset animation will be synchronized across all instances using the same MarqueeTextSync object.
Additional delay is added at the start of the animation, so waiting widgets will pause with the text at the beginning.

This provides the following advantages:

  • Reset animation is shared across all widgets to prevent chaotic jumping
  • The constant scrolling speed makes reading with multiple marquees on screen considerably easier
  • When waiting is required (for sync), the screen time for the beginning of the text is maximized, as it's often the most important part

Only visible MarqueeText widgets will be animated, ensuring animations won't be slowed down by off-screen MarqueeText components with very long text.

Use of MarqueeText in this PR

In this PR, MarqueeText is added in the following places:

  • Fullscreen Player Queue (For currently playing track, and on mouse hover)
  • Fullscreen Player (Title, Artist, and Album Name, with different MarqueeTextSync than the Queue)
  • Minimized Player (Artist and Title)
  • InfoHeader (when viewing an Album/Artist/Playlist/...)

Scrolling is done using CSS, while CPU usage seems to be minimal (tested using Firefox Profiler).

Screen Recordings of this PR in Action

Marquee.Fullscreen.Player.Phone.mp4
Marquee.Fullscreen.Player.Tablet.mp4
Marquee.Minimized.Player.and.Album.Info.mp4

@marcelveldt
Copy link
Member

I appreciate the PR and I think this addition will be liked by many.
However I do not like the fact that multiple elements are scrolling at the same time, which makes the screen super busy and distracting even. Also I think that the marquee could be a bit slower. Or maybe pause shortly between scrolls, like the Sonos app is doing.

I think for the full list of queue tracks we should only scroll the currently active item, and not the entire list.

@maximmaxim345
Copy link
Contributor Author

Thanks for the feedback.

I will make the scrolling slower (maybe half of the current speed). I will look into how SONOS handles it later, but this PR already pauses for 2 seconds at the end of the text before resetting. How about extending that to 4 seconds?

As for the Queue, I absolutely agree with you about the busyness (I mainly added the marquee to absolutely everything to see what works). As an alternative solution, I can propose: adjusting the 3-dot menu on a queue item to show the title/artist name in full with wrapping.

Keeping scrolling on for the currently active queue item sounds good. Then we can synchronize scrolling between the left and right side of the full-screen player (first video; currently, it was syncing separately due to different text lengths). This should further reduce busyness a bit.

Let me know what you think of this. I will try to adjust this PR in the coming week.

@marcelveldt
Copy link
Member

Yeah sounds good on all points.
As for the queue items I would indeed not let them all scroll (as that makes up for a super busy screen) but the suggestion to "reveal" the full title with a click of the contextmenu sounds good. Also, on desktop OS we can consider to scroll on mouse over.

Required for MarqueeText to not be pushed onto a new line
Previously a fixed width was used, which made MarqueeText not scroll to
the end on some screen sized.
Now a flexbox is used to ensure the title fits into the header.
This component allows the containing content to be automatically
scrolled. The key differentiator from other Marquee components is the
syncing capability for increased readability.
@maximmaxim345 maximmaxim345 force-pushed the feat/marquee branch 2 times, most recently from 51781de to 0d35907 Compare December 25, 2024 14:42
@maximmaxim345
Copy link
Contributor Author

Just updated this PR as discussed.
Also added your idea of scrolling when hovering with a mouse.

You can see the updated screen recordings above.

I also looked into how the SONOS app handles the marquee:
In my opinion, the main weakness of SONOS's approach is in cases where the album title is relatively short while the track title is long. In that case, for the majority of time, the album name is entirely hidden due to the scroll. This PR would show the beginning of the album name instead, which is considerably more useful than looking at a blank space.

As for revealing the full title in the context menu, I can add that in a separate PR later.

@marcelveldt
Copy link
Member

Perfect!

Copy link
Member

@marcelveldt marcelveldt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @maximmaxim345 !

@marcelveldt marcelveldt merged commit e3196d4 into music-assistant:main Dec 26, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants