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

Fix audio player overlapping on small screens #3018

Merged
merged 5 commits into from Feb 28, 2022

Conversation

thornbill
Copy link
Member

Changes
Fixes the audio player ui on small screens.

This is an alternative to #2972 and #2682 that uses flexbox to size items correctly.

Issues
N/A

@thornbill thornbill added this to Active PRs in Release 10.8.0 via automation Sep 30, 2021
@sonarcloud
Copy link

sonarcloud bot commented Sep 30, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@thornbill thornbill added the bug Something isn't working label Feb 22, 2022
@thornbill
Copy link
Member Author

This has an issue where the player and playlist controls overlap on small screens on iOS 10. I believe this is an issue with flex-shrink not working correctly in this version of Safari such that the album art is not properly scaled. imho this should probably not be considered a blocker since it seems to work correctly on any newer browser and iOS 10 usage should be pretty low at this point. We are also going to be dropping support for iOS 10 and 11 in the next app update (due to Expo no longer supporting those versions).

Screenshot 2022-02-23 at 00-08-40 iPhone 7 v10 3
(Screenshot is from an iPhone 7 running iOS 10 via BrowserStack)

Copy link
Contributor

@dmitrylyzo dmitrylyzo left a comment

Choose a reason for hiding this comment

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

Safari 10 seems to handle flexbox in a non-standard way (#3456).

UPD:
Also need to trim these (for remote video):

.nowPlayingPageImageContainer.nowPlayingPageImagePoster {
height: 50%;
overflow: hidden;
}
.nowPlayingPageImageContainer.nowPlayingPageImagePoster img {
height: 100%;
width: auto;
}

src/components/remotecontrol/remotecontrol.scss Outdated Show resolved Hide resolved
Copy link
Contributor

@dmitrylyzo dmitrylyzo left a comment

Choose a reason for hiding this comment

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

During video playback, the volume slider is overlapped by the buttons:

As a quick fix, remove this line:

Will be

src/components/remotecontrol/remotecontrol.scss Outdated Show resolved Hide resolved
Copy link
Contributor

@dmitrylyzo dmitrylyzo left a comment

Choose a reason for hiding this comment

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

There is a poster for cover-less audio.

.nowPlayingPageImageContainerNoAlbum {

I tried width: 80% for small media.

UPD:
But the icon looks too big on a small card - need to adjust the font size. In the next PR?

Btw, here is the custom CSS I use for debugging:

.nowPlayingInfoContainer{background-color:rgba(0,255,0,0.5)!important}
.nowPlayingPageImageContainer{background-color:rgba(255,0,0,0.5)!important}
.nowPlayingInfoControls{background-color:rgba(0,0,255,0.5)!important}
.nowPlayingInfoButtons{background-color:rgba(0,255,255,0.5)!important}
.nowPlayingPageImageAudio{opacity:0.5!important}

src/components/remotecontrol/remotecontrol.scss Outdated Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented Feb 28, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Release 10.8.0 automation moved this from Active PRs to Approved PRs Feb 28, 2022
@thornbill thornbill merged commit 76b633b into jellyfin:master Feb 28, 2022
Release 10.8.0 automation moved this from Approved PRs to Completed PRs Feb 28, 2022
@thornbill thornbill deleted the fix-small-player branch February 28, 2022 14:18
@phcreery
Copy link

This has an issue where the player and playlist controls overlap on small screens on iOS 10. I believe this is an issue with flex-shrink not working correctly in this version of Safari such that the album art is not properly scaled. imho this should probably not be considered a blocker since it seems to work correctly on any newer browser and iOS 10 usage should be pretty low at this point. We are also going to be dropping support for iOS 10 and 11 in the next app update (due to Expo no longer supporting those versions).

Screenshot 2022-02-23 at 00-08-40 iPhone 7 v10 3 (Screenshot is from an iPhone 7 running iOS 10 via BrowserStack)

Not sure if this is entirely related to outdated Safari client as it is reproducible in chrome desktop.
https://user-images.githubusercontent.com/44987569/119747193-ca46c580-be57-11eb-9106-91d2f5d03d03.png.

Regardless, I am happy to see a resolution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Release 10.8.0
  
Completed PRs
Development

Successfully merging this pull request may close these issues.

None yet

3 participants