-
-
Notifications
You must be signed in to change notification settings - Fork 66
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
Add native global menu support (macOS) #644
Conversation
63c47dc
to
f430d9c
Compare
Took a peek today, this seems too hard-coded. The shortcuts are only assigned on tree enter, and they don't seem to account for the user has changing the shortcuts. If necessary, you can add a custom notification for changed shortcuts in Utils.gd and listen to it here, fetching the first shortcut from each action instead. I don't know the new 4.3 functionality though, maybe that's not possible. |
37de696
to
3175e02
Compare
Needs to be updated after #711. Please check out the TranslationUtils class! |
a26b715
to
59b16bb
Compare
We're on a 4.3 beta now, this can be considered. (wait, what happened to this PR?) |
It looks like you somehow managed to edit a commit from way back in the history, and force-push, which modified all commits beyond that point too. Don't worry, I can fix it easily, that is, whenever I get around to working on this. |
Okay, thanks! Yeah, I was doing some weird things to delete a file from the git history (a file which is no longer in the repo or even relevant, but made 80% of its size). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TranslationUtils.get_shortcut_description()
should be used when it can. Also, there's a SHORTCUTS_CHANGED
custom notification now - the signal isn't needed. _action_call(action)
could also be replaced with ShortcutUtils.fn_call(action)
.
Could you also replace the remaining tr()
calls with TranslationServer.translate()
instead? We decided recently to always use this version because it can be used in static functions.
Also open the "update_translations.gd" script to update translations automatically after you're done.
09eb23c
to
69cb677
Compare
Done.
I'm not sure, it doesn't seem the same. Also, isn't a notification worse than a signal? A notification is propagated to everything, while a signal is only propagated to what connects to it.
I don't think so?
Done.
Done (and I discarded changes to the po/pot files made before running this script, so the changes to the po/pot files in this PR are purely from the script). |
|
||
|
||
func _setup_menu_items() -> void: | ||
# Included App and Help menus. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TranslationUtils should be used here to get the translated string of "open_settings". Same for the lines of code below. That way it's ensured that they are the same everywhere.
Thanks! |
Draft because I used the new Godot 4.3+ API so this will only work with Godot 4.3+. Follow-up to #643.
This is not just about clicking buttons in the menu, it also provides convenient documentation for the user of what actions are available and which have keys available to press.