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(useFullscreen): handle Safari iOS #2822

Merged
merged 2 commits into from Mar 14, 2023
Merged

fix(useFullscreen): handle Safari iOS #2822

merged 2 commits into from Mar 14, 2023

Conversation

ferferga
Copy link
Contributor

Description

Apple uses a different webkit function to enter fullscreen. See HTMLVideoElement and webkitEnterFullScreen documentation.

What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

bayramorhan
bayramorhan previously approved these changes Feb 28, 2023
@TianbinTobin
Copy link

const [REQUEST, EXIT, ELEMENT,, EVENT] = map doesn't match the [New WebKit] if

 [
    'webkitRequestFullscreen',
    'webkitEnterFullScreen',
    'webkitExitFullscreen',
    'webkitFullscreenElement',
    'webkitFullscreenEnabled',
    'webkitfullscreenchange',
    'webkitfullscreenerror',
  ]

@ferferga
Copy link
Contributor Author

ferferga commented Mar 1, 2023

@TianbinTobin You're right - That's what happens when you open PRs late at night 😅.
Just ammended my commit!

@antfu antfu merged commit 9d39495 into vueuse:main Mar 14, 2023
ferferga added a commit to jellyfin/jellyfin-vue that referenced this pull request Mar 21, 2023
* After the merge of vueuse/vueuse#2822, this is implemented upstream
ferferga added a commit to jellyfin/jellyfin-vue that referenced this pull request Mar 21, 2023
* After the merge of vueuse/vueuse#2822, this is implemented upstream
ferferga added a commit to jellyfin/jellyfin-vue that referenced this pull request Mar 21, 2023
* After the merge of vueuse/vueuse#2822, this is implemented upstream
ferferga added a commit to jellyfin/jellyfin-vue that referenced this pull request Mar 22, 2023
* After the merge of vueuse/vueuse#2822, this is implemented upstream
ferferga added a commit to jellyfin/jellyfin-vue that referenced this pull request Mar 22, 2023
* After the merge of vueuse/vueuse#2822, this is implemented upstream
ferferga added a commit to jellyfin/jellyfin-vue that referenced this pull request Mar 23, 2023
* After the merge of vueuse/vueuse#2822, this is implemented upstream
ferferga added a commit to jellyfin/jellyfin-vue that referenced this pull request Mar 23, 2023
* After the merge of vueuse/vueuse#2822, this is implemented upstream
ferferga added a commit to jellyfin/jellyfin-vue that referenced this pull request Mar 23, 2023
* After the merge of vueuse/vueuse#2822, this is implemented upstream
ferferga added a commit to jellyfin/jellyfin-vue that referenced this pull request Mar 23, 2023
* After the merge of vueuse/vueuse#2822, this is implemented upstream
@N3mezis
Copy link

N3mezis commented Mar 24, 2023

Hi there! Thanks for the work on this!

Unfortunately, this doesn't completely fix it. If target changes (i.e. null -> <video>), the map isn't updated, leading to REQUEST, EXIT, ELEMENT,, EVENT being invalid for the specific target.

@ferferga
Copy link
Contributor Author

@N3mezis Im aware of this. Initial work for properly handle it in this PR: #2904

My plan is to completely refactor this function afterwards. It didn't fully fix Safari iOS either, since some devices still rely on the deprecated (according to Apple docs) webkitEnterFullScreen (notice the case of S)

ferferga added a commit to jellyfin/jellyfin-vue that referenced this pull request Mar 28, 2023
* After the merge of vueuse/vueuse#2822, this is implemented upstream
ferferga added a commit to jellyfin/jellyfin-vue that referenced this pull request Mar 28, 2023
* After the merge of vueuse/vueuse#2822, this is implemented upstream
@ferferga
Copy link
Contributor Author

@N3mezis Check #2915

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants