Rough implementation of selected menu items for gtk backend

[longmathemagician]

If there's a way to make a gtk::prelude::IsA<gtk::MenuItem> + gtk::prelude::IsA<gtk::Widget> trait object then this could be a lot shorter, but I couldn't get that to work.

Technically checkbox menu entries should have a distinct unchecked state (at least in GTK), but I didn't see a distinction in druid's MenuItem which would allow for that so these do not.

[xarvic]

Technically checkbox menu entries should have a distinct unchecked state (at least in GTK), but I didn't see a distinction in druid's MenuItem which would allow for that so these do not.

I think it would make sense to change this in druid.

[longmathemagician]

The advantage of that would be conforming to GTK standards. I don't believe Windows or macOS employ visual distinction between standard and unchecked checkbox menu entries so it would only benefit linux builds.

The disadvantage is that the change will either break existing code or only weakly enforce the distinction (which IMO is worse than not having a distinction in the first place).

Thoughts?

[xarvic]

I am not sure i understand what you mean by "only weakly enforcing the distincition".

The disadvantage is that the change will [...] break existing code.

I agree, this would be a breaking change, at least at the druid-shell layer, but since druid is one of the only libraries depending on druid-shell this is probably fine.
In druid on the other hand we can just deprecate MenuItem::selected and MenuItem::selected_if and add a new method which accepts a closure that returns one of the three states.

[longmathemagician]

By "weakly enforce the distinction" I mean deprecating MenuItem::selected rather than just straight up removing it, with the short-term result being some druid programs will update and some will not. Whether or not that matters here is not my call to make, I'm just bringing it up because there are places where it does matter. Druid does not make a particularly strong attempt to use native controls and adhere to native platform conventions so I didn't think the value of doing so here would be particularly high.

In druid on the other hand we can just deprecate MenuItem::selected and MenuItem::selected_if and add a new method which accepts a closure that returns one of the three states.

I initially assumed you meant splitting the current MenuItem into a MenuItem and a SelectableMenuItem, but deprecating just MenuItem::selected should be sufficient since if the entry has a closure given by selected_if it's already been defined as some sort of SelectableMenuItem.

This is slightly off topic but MenuItem::selected_if drove me nuts, since if you're using it as a toggle and select that option then you've just modified your data which means the menu reopens (even jumping over to the new mouse location over the MenuItem you just clicked). Is there an example showing how it's supposed to be used?

[xarvic]

I thought about this again. Maybe we dont need to deprecate MenuItem::selected at all. We already have three states. We could do the following: If the selected closure is some, its return value chooses between selected and unselected. If there is no closure, the item is unselectable. This means changing MenuBuildCtx and druid-shells Menu, but the public api of druid will stay the same, which is good.

This is slightly off topic but MenuItem::selected_if drove me nuts, since if you're using it as a toggle and select that option then you've just modified your data which means the menu reopens (even jumping over to the new mouse location over the MenuItem you just clicked). Is there an example showing how it's supposed to be used?

What is the expected behaviour here? Should the Menu just close after selecting the option?

[longmathemagician]

I thought about this again. Maybe we dont need to deprecate MenuItem::selected at all. We already have three states. We could do the following: If the selected closure is some, its return value chooses between selected and unselected. If there is no closure, the item is unselectable. This means changing MenuBuildCtx and druid-shells Menu, but the public api of druid will stay the same, which is good.

Sure enough! That works beautifully. Just pushed a gtk version via 63f9562, will add fixes for the other platforms when I can test them.

This is slightly off topic but MenuItem::selected_if drove me nuts, since if you're using it as a toggle and select that option then you've just modified your data which means the menu reopens (even jumping over to the new mouse location over the MenuItem you just clicked). Is there an example showing how it's supposed to be used?

What is the expected behaviour here? Should the Menu just close after selecting the option?

That's certainly what I expected, but if not that then I would at least expect that the menu would stay open with only the checkmark updating.

[longmathemagician]

Still haven't had a chance to test anything beyond GTK, but this should at least build for the other platforms now.

I don't think macOS has a distinct unchecked MenuItem, and while I have seen them in Windows I don't believe they're a first party element. Might be worth adding in Windows along with MenuItem icons, at some point in the future unless someone really wants them now.

[xarvic]

Sorry, sliped

[longmathemagician]

No worries. Might have just fixed that x11 stub issue the test caught but like most of the other-platform commits here I'm doing it blind as I can't currently verify.

[xarvic]

I finally got some time to take a closer look. It works great, thanks! :)