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.

Sign up for GitHub

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

Jump to bottom

Add missing song information to players and apply EnableCoverAnimation to mobile player. #1268

Merged
merged 11 commits into from
Aug 17, 2021
Merged

Add missing song information to players and apply EnableCoverAnimation to mobile player. #1268

merged 11 commits into from
Aug 17, 2021

Conversation

mill1000
Copy link
Contributor

@mill1000 mill1000 commented Aug 1, 2021

Modifies the mobile and desktop players to add missing basic song info and create a more uniform look. The changes are (obviously) aligned with my preferences and may not fit the project's vision. I figured I'd at least put them out here as a point of discussion.

Mobile Player

  • Hides the original artist field and adds a new artist field with style matching song title.
  • Adds album and song year (if present) to player under artist field, with matching style.

New mobile player fields

Mobile player with cover animations disabled

Desktop Player

  • Song year is added to song info when present in the format of "Artist - Album - Year".

Desktop player with song year

@certuna
Copy link
Contributor

certuna commented Aug 2, 2021 •
edited

I like it! Album + Year info is useful I think. For what it's worth, Plexamp also has Album Title (Year) on the Now Playing screen which looks/works great (note: they've removed the stars/bitrate line from the now-playing screen and added year in parentheses, this screenshot is from an older version)

And on mobile you'd want to save as many cpu cycles as you can re: battery life.

Have you tested this with non-square (i.e. rectangular) images?

@mill1000
Copy link
Contributor Author

mill1000 commented Aug 3, 2021

Thanks!

Just checked with a rectangular cover art, and the behavior could be improved. The lower half stretches to fill the space which causes the buttons to move. That's not ideal. I'll see what I can do about that.

Rectangular cover art

Frankly, the standard behavior isn't that great either, but at least the buttons stay the same.
stock rectangular

@mill1000
Copy link
Contributor Author

mill1000 commented Aug 3, 2021

Tried fixing the behavior for rectangular album art for both animated and static mobile players.

Here are the results:

Static
Larger Y than X
Screen Shot 2021-08-02 at 20 33 31

Larger X than Y
Screen Shot 2021-08-02 at 20 33 35

Animated
Larger Y than X
Screen Shot 2021-08-02 at 20 33 11

Larger X than Y
Screen Shot 2021-08-02 at 20 33 08

@mill1000
Copy link
Contributor Author

mill1000 commented Aug 3, 2021

And similarly for the desktop player.

Old behavior
Y larger than X

X larger than Y

New behavior
Y larger than X

X larger than Y

@deluan
Copy link
Member

deluan commented Aug 3, 2021

Thanks for the PR @mill1000 ! I'll take a look

@deluan
Copy link
Member

deluan commented Aug 15, 2021 •
edited

Hey @mill1000, is this ready for review? Is there anything missing? Looks good to me for merging. Let me know as I want to include this in the next release.

@mill1000 mill1000 marked this pull request as ready for review August 17, 2021 17:46
@mill1000
Copy link
Contributor Author

I have no further changes in mind. I've marked it ready to review. Hopefully it's not too late the for the next release.

@deluan deluan merged commit aa72d3d into navidrome:master Aug 17, 2021
@mill1000 mill1000 deleted the issues/mobile_player_revamp branch August 23, 2021 19:42
samarsault pushed a commit to samarsault/navidrome that referenced this pull request Sep 5, 2021
@mill1000 @samarsault
Add missing song information to players and apply EnableCoverAnimatio…
1246cd0 
…n to mobile player. (navidrome#1268)

* Disable mobile player cover animation when EnableCoverAnimation is set to false. Also increase cover art size and remove rounded borders.

* Display song album and year in mobile player view

* Remove default singer element from mobile player and reduce vertical white space

* Only add song year if it exists

* Add song year to desktop player when present

* Increase non-animated cover size to 85% and set a limit on the width of 600px.

* Explain what what the styles impact

* Remove unused style for songArtist

* Apply prettier

* Adjust player styles to handle nonsquare album art better. Should probably push this upstream too

* Also fix desktop player's handling of non square cover art.
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Mobile web interface animates cover art when EnableCoverAnimation is set to false
3 participants
@mill1000 @certuna @deluan