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

feat(macos): Fix with_titlebar_transparent, add WindowExtMacOS::set_titlebar_transparent #372

Closed
wants to merge 7 commits into from

Conversation

probablykasper
Copy link
Member

@probablykasper probablykasper commented Apr 22, 2022

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Docs
  • New Binding issue #___
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes, and the changes were approved in issue #___
  • No

Checklist

  • When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
  • A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.
  • I have added a convincing reason for adding this feature, if necessary

Other information

Used @JonasKruckenberg's code from tauri-apps/tauri#2663, and it looks like it works!

Fixes with_titlebar_transparent by setting a style mask, and adds WindowExtMacOS::set_titlebar_transparent

@JonasKruckenberg
Copy link
Contributor

there is also with_titlebar_transparent already, which (I think?) was supposed to do this already, but doesn't. Maye we could just fix with_titlebar_transparent with this implementation? 🤔

@probablykasper
Copy link
Member Author

there is also with_titlebar_transparent already, which (I think?) was supposed to do this already, but doesn't. Maye we could just fix with_titlebar_transparent with this implementation? 🤔

Ah, looks like you're right!

I made it so with_titlebar_transparent is what's used, and I also made it not hide the title because there's a separate option for that.

@probablykasper probablykasper changed the title feat(macos): Hide titlebar, but not window controls feat(macos): Fix titlebar_transparent Apr 25, 2022
@probablykasper probablykasper changed the title feat(macos): Fix titlebar_transparent feat(macos): Fix with_titlebar_transparent, add WindowExtMacOS::set_titlebar_transparent Apr 25, 2022
@probablykasper probablykasper marked this pull request as ready for review April 25, 2022 09:52
@probablykasper probablykasper requested a review from a team April 25, 2022 09:52
@probablykasper probablykasper requested a review from a team as a code owner April 25, 2022 09:52
Comment on lines +208 to +214
let mut style_mask = ns_window.styleMask();
style_mask.set(
NSWindowStyleMask::NSFullSizeContentViewWindowMask,
true,
);
ns_window.setStyleMask_(style_mask);

Copy link
Member

Choose a reason for hiding this comment

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

There is WindowBuilderExtMacOS::with_fullsize_content_view for exactly doing this. Users should just use it.

Suggested change
let mut style_mask = ns_window.styleMask();
style_mask.set(
NSWindowStyleMask::NSFullSizeContentViewWindowMask,
true,
);
ns_window.setStyleMask_(style_mask);

Comment on lines +1247 to +1253
let id = self.ns_window() as cocoa::base::id;
let mut style_mask = id.styleMask();
style_mask.set(
NSWindowStyleMask::NSFullSizeContentViewWindowMask,
transparent,
);
self.ns_window.setStyleMask_(style_mask);
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be the responsibility of this method, it should probably be split into another method WindowExtMacOS::set_fullsize_content_view

"tao": patch
---

Fix macOS `WindowBuilderExtMacOS::with_titlebar_transparent`
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it was broken in the first place, because it seems like users needed to just pair it with WindowBuilderExtMacOS::with_fullsize_content_view in wry/tauri apps.

@probablykasper
Copy link
Member Author

Alright, looks like tao already supports what would be needed for making titlebars transparent:
with_titlebar_transparent + with_fullsize_content_view + with_title_hidden

@amrbashir amrbashir deleted the macos-hidden-titlebar branch April 25, 2022 18:27
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

3 participants