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

Cross-platform menu issues (for Runebender) #1306

Open
raphlinus opened this issue Oct 13, 2020 · 7 comments
Open

Cross-platform menu issues (for Runebender) #1306

raphlinus opened this issue Oct 13, 2020 · 7 comments
Labels
discussion needs feedback and ideas shell concerns the shell abstraction

Comments

@raphlinus
Copy link
Contributor

I'm doing most my Runebender development on Windows, and have come across some issues related to menus. This is a tracking issue for those.

  • The Runebender source uses platform_menus::mac menus directly, without any kind of configuration check. Those use RawMods::Meta, which bind to Windows, and that doesn't work.

  • The shortcuts ("Ctrl-S") are not displayed in Windows menus. I think that was intended for a while, but I didn't quite get around to implementing them. There was churn around HotKey but I think that's stabilized reasonably well now thanks to the keyboard work.

  • The provided menus are missing alt-key shortcuts. These appear to work, by adding an ampersand in the localized string (&File, E&xit, etc), and are stripped out (at least on mac, haven't tested Linux yet).

The most expedient fix to the first problem is to switch from RawMods to SysMods::Cmd. That would result in reasonable behavior if menus for the wrong platform were used, if not the most platform-appropriate keybindings. A deeper change would be to expand the platform_menus::common choices, and more aggressively enforce the use of actual platform specific menus.

I have local patches with expedient fixes for most of this, and plan to PR them. I'm open to discussion about the right way to proceed, and of course if people want to take on more extensive rework.

@raphlinus raphlinus added discussion needs feedback and ideas shell concerns the shell abstraction labels Oct 13, 2020
@kud1ing
Copy link
Contributor

kud1ing commented Oct 13, 2020

Not sure whether it fits here, but on Mac the "Quit" entry should be last. While on Mac Rundender it is the 3rd last.

Bildschirmfoto 2020-10-13 um 20 57 41

@raphlinus
Copy link
Contributor Author

raphlinus commented Oct 13, 2020

Technically, that's a Runebender issue, but it's relevant to discuss here, as the point is how we can best support a native-feeling menu structure appropriate to each platform.

ETA: actually I think that might be a deeper issue. I haven't investigated fully, but I believe the reason those are showing up on the application menu is that they are not included in the Edit menu, which I believe is where they should be. Of course, it's also the case that those two items won't work until we do IME, but that's another kettle of fish (and one for which I think we should have a tracking issue).

@kud1ing
Copy link
Contributor

kud1ing commented Oct 13, 2020

they are not included in the Edit menu, which I believe is where they should be.

Indeed, you're right about this.

@kud1ing
Copy link
Contributor

kud1ing commented Oct 13, 2020

Another thing is that it should be "runebender" instead of "Druid" obviously.

@raphlinus
Copy link
Contributor Author

Indeed. That's because -app-name is set in resources/en-US/builtin.ftl. I'm not familiar enough with Fluent (or perhaps it's our ResourceManager) to know how easy it is to override that from the app.

@kud1ing
Copy link
Contributor

kud1ing commented Oct 13, 2020

Regarding the application menu on Mac: wxWidgets and Qt both appear to do the right thing for the developer.

@cmyr
Copy link
Member

cmyr commented Oct 13, 2020

Those two misplaced items are being added for us by AppKit at some point, I'll have to look into that.

raphlinus added a commit that referenced this issue Oct 14, 2020
This patch adds a string to describe hotkeys for menu items in the
Windows back-end.

Part of work tracked in #1306
raphlinus added a commit that referenced this issue Oct 21, 2020
If an application were to misuse a platform-specific for the wrong
platform, it would get unusable keybindings. With this patch, it might
not be the most appropriate for the platform, but it would work.

The linked bug below suggests a more systematic approach, but this is a
minimal patch to get things to a less broken state.

Part of work tracked in #1306.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion needs feedback and ideas shell concerns the shell abstraction
Projects
None yet
Development

No branches or pull requests

3 participants