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 player full screen causing the app to start in full screen #5138

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

absidue
Copy link
Member

@absidue absidue commented May 20, 2024

Fix player full screen causing the app to start in full screen

Pull Request Type

  • Bugfix

Related issue

Description

This is an alternative to #4649 that should hopefully work more consistently (the creator of that pull request is aware that I am making this one).

When the app is closed while in full screen regardless of whether only the player or the entire window was full-screened, we save the full screen state and restore it at start up. For people that full-screened the app that is great, but for people that full-screened the player it's less desirable to have the entire app start in full screen.

Testing

Full screen doesn't persist if player is full-screened

  1. Open a video
  2. Full screen the player, either through the button or the F hotkey
  3. Close the app e.g. Ctrl+Q or Alt+F4
  4. Open the app again and make sure it launches with normal window dimensions

Full screen persists if you full screen the window (regression test)

  1. Full screen the window e.g. F11
  2. Close the app e.g. Ctrl+Q or Alt+F4
  3. Open the app again and make sure it launches in full screen.

Desktop

  • OS: Windows
  • OS Version: 10
  • FreeTube version: 0.20.0

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) May 20, 2024 20:47
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label May 20, 2024
Copy link
Member

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc left a comment

Choose a reason for hiding this comment

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

Full screen doesn't persist if player is full-screened

  1. Open a video
  2. Full screen the player, either through the button or the F hotkey
  3. Close the app e.g. Ctrl+Q or Alt+F4
  4. Open the app again and make sure it launches with normal window dimensions

Works!

Full screen persists if you full screen the window (regression test)

  1. Full screen the window e.g. F11
  2. Close the app e.g. Ctrl+Q or Alt+F4
  3. Open the app again and make sure it launches in full screen.

Works!

Full screen persists if you full screen the window and if player is fullscreened

  1. Full screen the window e.g. F11
  2. Full screen the player, either through the button or the F hotkey
  3. Close the app e.g. Ctrl+Q or Alt+F4
  4. Open the app again and make sure it launches in full screen.

App unfortunately doesn't launch in fullscreen, tested on win10

@github-actions github-actions bot added PR: merge conflicts / rebase needed and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels May 20, 2024

This comment was marked as outdated.

Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

Copy link
Contributor

This PR is stale because it has been open 28 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@efb4f5ff-1298-471a-8973-3d47447115dc

doesnt this also address #3220?

Copy link

@iacore iacore left a comment

Choose a reason for hiding this comment

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

LGTM

src/main/index.js Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants