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

macOS: titlebar in borderless fullscreen fix #2046

Closed
wants to merge 1 commit into from

Conversation

tinaun
Copy link
Contributor

@tinaun tinaun commented Nov 1, 2021

Fixes #1195, but does not implement the user-facing changes talked about in that issue (should we create a new issue for that feature?)

  • Tested on all platforms changed
  • Compilation warnings were addressed
  • cargo fmt has been run on this branch
  • cargo doc builds successfully
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

@tinaun
Copy link
Contributor Author

tinaun commented Nov 2, 2021

I didn't realize there was a 2-year old pr doing much the same thing (#1326) also sitting in waiting-for-review purgatory when I wrote this pr.

I didn't include any logic for handling the borderless fullscreen <-> exclusive fullscreen in this pr, because you can get around it in user code easier than you can work around "pressing the fullscreen button on a window prevents you from leaving unless you close the program"

@francesca64 francesca64 self-requested a review November 2, 2021 22:16
@francesca64
Copy link
Member

@tinaun which PR do you think would be more ideal to merge, assuming both are equally likely to be accepted?

@tinaun
Copy link
Contributor Author

tinaun commented Nov 2, 2021

that pr has a bug with its handling of the exclusive -> borderless logic that would need to be included in the pull request if you wanted to merge that one (it might be fixed in the branch mentioned here, i haven't tested it #1326 (comment))

@francesca64
Copy link
Member

@tinaun gotcha, thanks for the insight.

@irh would you still be interested in helping push this forward?

@bschwind
Copy link

bschwind commented Nov 3, 2021

Hi, how can I help test this? Is it enough to run the window example with cargo run --example window, go into borderless fullscreen with the green maximize button, and verify the menu bar is still available when moused over?

@irh
Copy link
Contributor

irh commented Nov 3, 2021

@francesca64 Yes I'd be happy to help out here, although it's not clear to me what needs to be done? I think #1326 feels complete with my extra commit in place, but I'm no expert in windowing (I just found the PR and figured I'd see if I could help) so I could be missing something. I'd also be happy to see this PR get landed to solve the main problem of fullscreen on macOS and then leave the mode switching behaviour to be refined in a followup.

@francesca64
Copy link
Member

@irh you wrote a detailed commit description, so that makes you automatically reputable! This is a small enough change that I don't think there's any need for us to split it up into multiple passes. Your branch is the most complete, so if you open a PR for it, we can move forward with merging it!

@bschwind thanks for the interest! Doing that will test the most common case, though the edge case of switching between exclusive and borderless fullscreen is the thornier aspect to this.

@irh
Copy link
Contributor

irh commented Nov 4, 2021

OK thanks @francesca64 =) I've opened #2053.

@francesca64
Copy link
Member

Subsumed by #2053

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

Successfully merging this pull request may close these issues.

Fullscreen Titlebar Won't Show on macOS
4 participants