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

Remove fullscreen button on iPad if using a non Safari browser #334

Merged
merged 7 commits into from
Nov 29, 2024

Conversation

lukehb
Copy link
Contributor

@lukehb lukehb commented Nov 28, 2024

Relevant components:

  • Frontend UI library

Problem statement:

#219

Using fullscreen video on iPad + touch interactions + non-Safari browsers = video gets dragged around the screen.

This is fairly unusable for Pixel Streaming but it browser defined behaviour that can't be disabled it seems.

Additionally if you leave fullscreen from one of these non-Safari browsers on the iPad the video stream will pause and cannot be resumed without re-entering fullscreen mode because we hide the video controls.

Solution

Don't allow fullscreen button on these non-Safari browsers on iPad.

Documentation

N/A

Test Plan and Compatibility

Test on iPad on Safari = button still shows up
Test on iPad on Chrome = button is gone

@lukehb lukehb added auto-backport Used to specify we want a PR to auto backport to a branch, must be paired with auto-backport-to-UEX. auto-backport-to-UE5.4 Auto backport to UE 5.4 auto-backport-to-UE5.3 Auto backport to UE 5.3 auto-backport-to-UE5.2 Auto backport to UE 5.2 labels Nov 28, 2024
@lukehb lukehb removed auto-backport Used to specify we want a PR to auto backport to a branch, must be paired with auto-backport-to-UEX. auto-backport-to-UE5.4 Auto backport to UE 5.4 auto-backport-to-UE5.3 Auto backport to UE 5.3 auto-backport-to-UE5.2 Auto backport to UE 5.2 labels Nov 28, 2024
@@ -21,7 +21,7 @@
</head>

<!-- The Pixel Streaming player fills 100% of its parent element but body has a 0px height unless filled with content. As such, we explicitly force the body to be 100% of the viewport height -->
<body style="width: 100vw; height: 100svh; min-height: -webkit-fill-available; font-family: 'Montserrat'; margin: 0px">
<body style="width: 100vw; height: 100svh; min-height: -webkit-fill-available; overflow: hidden; overscroll-behavior: none; font-family: 'Montserrat'; margin: 0px">
Copy link
Contributor Author

@lukehb lukehb Nov 28, 2024

Choose a reason for hiding this comment

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

This removes scroll bounce zone on iPad that is unhelpful for Pixel Streaming

@lukehb lukehb added auto-backport Used to specify we want a PR to auto backport to a branch, must be paired with auto-backport-to-UEX. auto-backport-to-UE5.4 Auto backport to UE 5.4 auto-backport-to-UE5.3 Auto backport to UE 5.3 auto-backport-to-UE5.2 Auto backport to UE 5.2 labels Nov 28, 2024
@lukehb lukehb linked an issue Nov 28, 2024 that may be closed by this pull request
@mcottontensor mcottontensor changed the base branch from UE5.5 to master November 29, 2024 00:09
@mcottontensor mcottontensor merged commit 3f10f01 into master Nov 29, 2024
19 checks passed
@mcottontensor mcottontensor deleted the issue219 branch November 29, 2024 00:10
Copy link
Contributor

💔 All backports failed

Status Branch Result
UE5.4 Backport failed because of merge conflicts
UE5.3 Backport failed because of merge conflicts
UE5.2 Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

backport --pr 334

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

mcottontensor added a commit to mcottontensor/PixelStreamingInfrastructure that referenced this pull request Nov 29, 2024
Remove fullscreen button on iPad if using a non Safari browser

(cherry picked from commit 3f10f01)

# Conflicts:
#	Frontend/implementations/EpicGames/src/player.html
#	Frontend/ui-library/src/Application/Application.ts
mcottontensor added a commit to mcottontensor/PixelStreamingInfrastructure that referenced this pull request Nov 29, 2024
Remove fullscreen button on iPad if using a non Safari browser

(cherry picked from commit 3f10f01)

# Conflicts:
#	Frontend/ui-library/src/Application/Application.ts
mcottontensor added a commit to mcottontensor/PixelStreamingInfrastructure that referenced this pull request Nov 29, 2024
Remove fullscreen button on iPad if using a non Safari browser

(cherry picked from commit 3f10f01)

# Conflicts:
#	Frontend/ui-library/src/Application/Application.ts
@mcottontensor
Copy link
Collaborator

💚 All backports created successfully

Status Branch Result
UE5.2
UE5.3
UE5.4

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

mcottontensor added a commit that referenced this pull request Nov 29, 2024
[UE5.2] Merge pull request #334 from EpicGamesExt/issue219
mcottontensor added a commit that referenced this pull request Nov 29, 2024
[UE5.3] Merge pull request #334 from EpicGamesExt/issue219
mcottontensor added a commit that referenced this pull request Nov 29, 2024
[UE5.4] Merge pull request #334 from EpicGamesExt/issue219
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Used to specify we want a PR to auto backport to a branch, must be paired with auto-backport-to-UEX. auto-backport-to-UE5.2 Auto backport to UE 5.2 auto-backport-to-UE5.3 Auto backport to UE 5.3 auto-backport-to-UE5.4 Auto backport to UE 5.4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] - Video freeze on chome ipad after exiting fullscreen
2 participants