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

Add ability to disable title bar & OSC via enableTitleBarAndOSC pref key #5044

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

Conversation

svobs
Copy link
Contributor

@svobs svobs commented Jul 4, 2024


Description:

  • Adds new Preference.Key: .enableTitleBarAndOSC, default true
  • Minor reordering of OSC-related pref keys to make more sense, and update some of their comments which fell out of date (sorry, couldn't help myself), as well as making it clearer how some of them work together
  • Modify setupOnScreenController and logic while entering & exiting full screen to support the new pref
  • Add observer for the new key to MainWindowController so that setupOnScreenController is called as soon as it is changed

This PR only adds the feature itself and its pref entry. I'll submit another PR which adds controls for it to the Settings UI. For those who want to test this for the time being, they can enter the following in the Terminal:

# Disable OSC
defaults write com.colliderli.iina enableTitleBarAndOSC 0
# Enable OSC
defaults write com.colliderli.iina enableTitleBarAndOSC 1

@low-batt
Copy link
Contributor

low-batt commented Jul 4, 2024

I noticed it does not include suppressing the title. For the use case we are targeting it should also suppress the title. Or we could control the title separately. Or maybe not show it upon mouse movement unless the mouse is at the top of the screen where the title would be. Like the Mac menu bar behaves when in full screen mode.

@lhc70000
Copy link
Member

lhc70000 commented Jul 5, 2024

Continue from #4553...

Is this feature implemented to solve #4553? If so, then I think it's better to make this a runtime property rather than a pref key, because the user probably only wants to disable the OSC when watching the movie. It would be a hassle if they must open the settings window to enable/disable this feature. And there's little reason to remember this setting permanently.

@svobs
Copy link
Contributor Author

svobs commented Jul 6, 2024

Is this feature implemented to solve #4553?

I probably should have filled an issue which better fit this PR...
@lhc70000 I'll respond in the comments for #4553.

@svobs svobs marked this pull request as draft July 8, 2024 07:32
@svobs svobs changed the title Add ability to disable OSC via enableOSC pref key Add ability to disable title bar & OSC via enableTitleBarAndOSC pref key Jul 25, 2024
@svobs svobs marked this pull request as ready for review July 25, 2024 08:14
@svobs
Copy link
Contributor Author

svobs commented Jul 25, 2024

As noted in #4553, I've refactored so that the setting now covers both title bar & OSC.

It's designed to co-exist with disableUI, so that in effect either setting will disable the "fadeable views" in the window. I noticed that disableUI has some quirks. For example, when disableUI is true, the OSD is still shown with a vertical offset which is influenced by the size of the top bar, so if the OSC is configured to be at "top", the OSD will be lower, like this:
SCR-20240725-cgap

I decided to use a constant vertical offset when enableTitleBarAndOSC is false which matches the title bar height instead, like this:
SCR-20240725-cdld

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.

3 participants