feat: WIP Custom menu builder

[lemarier]

• 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

[adamnemecek]

Yes, let’s make this happen.

[maroider]

I understand that this was asked for in #1583, but an API like this runs counter to Winit's current stated scope.

From FEATURES.md:

Winit does not directly expose functionality for drawing inside windows or creating native
menus, but does commit to providing APIs that higher-level crates can use to implement that
functionality.

I wasn't there when this decision was made, but I'd like to see a strong argument as to why this must be implemented inside Winit, rather than having it live in a separate crate. From what I can tell (and have been told), the way Winit currently forwards events, plus the way the native macOS APIs work, should enable anyone to add their own native macOS menus without modifying Winit.

Additionally, this touches the cross-platform API, so what other platforms can do must be taken into consideration.
Windows does have native menus, but Winit only facilitates them through WindowBuilderExtWindows::with_menu due to how Windows needs to know if we're adding a menu before the window itself is created in order to create the window correctly.

AFAIK, Linux (X11 & Wayland, really) has no real notion of native menus, and there are two major GUI libraries which are generally used to provide a "native" look and feel: GTK and QT. If we tried supporting this on Linux, then we'd probably have to link against GTK and QT dynamically, and that doesn't even get into how these frameworks tend to want to control the windows themselves.

cc @casperstorm @LoganDark @madsmtm (you guys seem like you know a thing or two about macOS)
cc @ArturKovacs

[madsmtm]

I agree fully with @maroider - though I know this is a very desired feature (!), it is a hard problem to tackle, and not one I think is appropriate for winit (which has a tough enough life as-is).

Also, a proper menu library would not only handle the menu bar, but also context menus and the dock menu.

See also a previous issue on this, #427, which was closed due to being "out of scope".

(Background: I'm privately slowly working on a separate crate for exactly this, hence the reason I made WindowBuilderExtWindows::with_menu, and I'll definitely be borrowing from this nice PR if @lemarier will let me 😉)

[adamnemecek]

What's the reasoning for why this is a bad idea? That Linux doesn't support it? Who wrote the FEATURES.md file by the way?

I think that this is a feature that a non-trivial percentage of the users are expecting. Putting it in a separate crate will cause fragmentation and like reimplementing the same things over and over.

I think it makes sense to do it in winit considering for example how on macOS, you have to do some black magic to get the macOS menu working on the first start (#1918).

The feature is small enough that I think it has a place in winit.

@madsmtm what about this feature is hard? Druid-shell has this menus. Context menus are something else entirely. Those are not a part of the windowing per se.

[lemarier]

Hey guys!

We had a test PR in wry our window management library built on top of winit for tauri.

Since this PR prevents us to register a menu correctly as your menu is always registered after. Didn't want to make a tweak to cover it. I thought it'll be great to make it for the whole project instead of trying to tweak all windows inside our app.

Also, your menu didn't cover basic functions like copy paste etc... also, it can't be disabled.

It's why I thought a Windows / macOS version would be great.
Also. the main reason I wanted to make it a winit feature is we could get it from the event loop
This way we have access to the window and the control flow who is really good.

Otherwise, we need to make a lot of tweaks, etc... and I think it would personally be beneficial for the whole community.

I personally think this is a major issue that should be addressed.

I'm open to any discussion and would really love to see the project moving forward but personally, I'm really scared...
We have the plan to contribute a lot in the future to the project but please bear with me and understand the situation.

Anyways..... not here to fight if it's good or not.

[maroider]

@adamnemecek

What's the reasoning for why this is a bad idea? That Linux doesn't support it?

It's because the Winit project has a (somewhat) well-defined scope.
It manages windows and events from input devices (and other sources of window events).

Winit tries to avoid adding features which can be implemented outside of Winit, and the consensus seems to be that native menus is one such feature.

Who wrote the FEATURES.md file by the way?

Mostly the Winit maintainers at the time it was written, it seems. See #695.

Putting it in a separate crate will cause fragmentation and like reimplementing the same things over and over.

I'm fine with that. I'd also be fine with there being such a crate under the rust-widowing umbrella, but that's not my call to make.

Druid-shell has this menus.

Winit isn't druid-shell. druid-shell exists in part because Raph Levien didn't believe that the approach Winit is taking would suit the Druid project. druid-shell doesn't try to be Winit either (to my knowledge).

[madsmtm]

I'm sorry if I came across as aggressive, definitely not my intention! I was simply just trying to weigh in with my opinion 😁.

@adamnemecek

The relevant part of FEATURES.md was written in #695 - that does not mean it's set in stone, the scope can be extended, but it does mean that we'll have to take our time to think about it!

The primary reasoning, I think, is that it can be done outside winit - which I think, if done well, will lead to less overall ecosystem fragmentation, since we could make a single crate that druid and similar could use as well. Also, what I was trying to say with the context menus is that their API in Cocoa Appkit lies so close that if you're implementing one, you might as well implement the other.

True, we did have to do some black magic to get menus to work, but that was clearly a bug in winit; now they work, so this should be a non-issue.

@lemarier

Thanks for the background information, always helps!

I did raise the concern that the menu couldn't be disabled, see #1583 (review), so if that's a real issue multiple people are having we could add a switch for it in winit. But you should be able to register your own custom menu in the Event::NewEvents(StartCause::Init) event?

Sorry that you're scared, Open Source shouldn't be that! We'll talk about it as humans 😉

[maroider]

Welp, I was going to write more, but @madsmtm just about covered what I wanted to cover.

Anyway, @lemarier, I didn't come with the intention to be some scary person blocking progress, and I'm sorry if I made you feel that way. Getting involved with Winit for the first time was a daunting experience for me, so I can imagine that it is discouraging to have your first PR against the repo be criticized (not quite sure it this is the right word).

[lemarier]

I'll close the PR as it's out of scope for winit.
I hope you guys will add the option to disable the menu on macOS.
I'll work on an external library as suggested.

[casperstorm]

First of all, good job @lemarier. I think it's a tough decision of what fits into winit but I think @maroider and @madsmtm hits the nail on the head here:

it is a hard problem to tackle, and not one I think is appropriate for winit (which has a tough enough life as-is).

And I agree @maroider suggestion. I think this would fit well:

I'd also be fine with there being such a crate under the rust-widowing umbrella, but that's not my call to make.

@madsmtm , Regarding:

I did raise the concern that the menu couldn't be disabled, see #1583 (review), so if that's a real issue multiple people are having we could add a switch for it in winit. But you should be able to register your own custom menu in the Event::NewEvents(StartCause::Init) event?

IMHO we should be able to disable it when we have an alternative way of adding a menu, eg. with a separate crate or similar.