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

[V2] Mac: Cannot show menus correctly #1588

Closed
cxt90730 opened this issue Jul 18, 2022 · 9 comments
Closed

[V2] Mac: Cannot show menus correctly #1588

cxt90730 opened this issue Jul 18, 2022 · 9 comments
Labels
awaiting feedback More information is required from the requestor

Comments

@cxt90730
Copy link

Description

Cannot show menus correctly on mac. Location and label display is not what I expect. I don't know if the document needs to be updated? (e.g. runtime.Quit now needs ctx of lifecycle, but document of Menus do not show this.)

To Reproduce

Mycode:

        AppMenu := menu.NewMenu()
	FileMenu := AppMenu.AddSubmenu("File")
	FileMenu.AddText("WindowSetTitle", keys.CmdOrCtrl("1"), func(_ *menu.CallbackData) {
		runtime.WindowSetTitle(ctx, time.Now().Local().String())
	})
	FileMenu.AddSeparator()
	FileMenu.AddText("WindowCenter", keys.CmdOrCtrl("2"), func(_ *menu.CallbackData) {
		runtime.WindowCenter(ctx)
	})
	FileMenu.AddSeparator()
	FileMenu.AddText("WindowSetLightTheme", keys.CmdOrCtrl("3"), func(_ *menu.CallbackData) {
		runtime.WindowSetLightTheme(ctx)
	})
	FileMenu.AddText("WindowSetDarkTheme", keys.CmdOrCtrl("4"), func(_ *menu.CallbackData) {
		runtime.WindowSetDarkTheme(ctx)
	})
	FileMenu.AddSeparator()
	FileMenu.AddText("WindowGetSize", keys.CmdOrCtrl("5"), func(_ *menu.CallbackData) {
		fmt.Println(runtime.WindowGetSize(ctx))
	})
	if os_runtime.GOOS == "darwin" {
		AppMenu.Append(menu.AppMenu())
		AppMenu.Append(menu.EditMenu()) // on macos platform, we should append EditMenu to enable Cmd+C,Cmd+V,Cmd+Z... shortcut
	}
	runtime.MenuSetApplicationMenu(ctx, AppMenu)

Uploading image.png…

Expected behaviour

display correct

Screenshots

No response

Attempted Fixes

No response

System Details

unnecessary...

Additional context

No response

@cxt90730
Copy link
Author

image

@cxt90730
Copy link
Author

emmm, I need to add these code before:

         AppMenu := menu.NewMenu()
	if os_runtime.GOOS == "darwin" {
		AppMenu.Append(menu.AppMenu())
	}

because Mac default occupied the first position of the Menuitem, and cannot change the label name

@leaanthony
Copy link
Member

So is it that the docs need improving? It's not clear what's happening with this ticket 👍

@leaanthony leaanthony added the awaiting feedback More information is required from the requestor label Jul 18, 2022
@acheong08
Copy link
Contributor

The documentation clearly includes

if runtime.GOOS == "darwin" {
    AppMenu.Append(menu.EditMenu())  // on macos platform, we should append EditMenu to enable Cmd+C,Cmd+V,Cmd+Z... shortcut
}

under Reference > Menus

@leaanthony
Copy link
Member

Great. Can this be closed then?

@cxt90730
Copy link
Author

@acheong08 @leaanthony Sorry, I may not express it clearly. According to the document, I cannot show the submenu File , because Mac default occupied the first position of the Menuitem, and cannot change the menu's label name. I must write code like this way first :

 AppMenu := menu.NewMenu()
	if os_runtime.GOOS == "darwin" {
		AppMenu.Append(menu.AppMenu())
	}

and then, I can add other menu that I customized or menu.EditMenu(), otherwise, the display will not be correct.

P.S. runtime.Quit() now is need arg ctx, so the example in Reference > Menus is wrong...

@acheong08
Copy link
Contributor

acheong08 commented Jul 19, 2022

Confirmed on MacOS 12.4. The first Menu will always be the Application menu on MacOS, documentation needs to be updated.

runtime.Quit() needs ctx which cannot be easily obtained in main.go. It is better to create a Quit function in app.go and set cmd+q to a.Quit()

acheong08 referenced this issue Jul 19, 2022
MacOS always sets the first menu item to the application name. A blank AppMenu should thus be appended to mitigate the issue of displaced menus.

runtime.Quit() now requires context.Context. Pointing to a Quit function in `app.go` is easier.
@leaanthony
Copy link
Member

Happy to accept a PR for your commit 👍

leaanthony pushed a commit that referenced this issue Jul 19, 2022
* Update example for macOS compatibility

Reference #1588

* Add comments, remove duplicates
@cxt90730
Copy link
Author

Awesome, thanks~

leaanthony added a commit that referenced this issue Jul 21, 2022
* Add troubleshooting for `Not Found` error (#1586)

Issues with Vite's optimizer

* Remove usage of unsafe.Pointer in winc (#1556)

* Remove usage of unsafe.Pointer

* [windows] Remove MakeIntResource and add overloads for Load functions

* Fix `EnumProc` race condition.

* Refactor `EnumDisplayMonitors` to use `unsafe.Pointer` instead of `uintptr`

Co-authored-by: stffabi <stffabi@users.noreply.github.com>

* [windows] Fix stack corruption when using ICoreWebView2HttpHeadersCollectionIterator (#1589)

The API expects a `BOOL *` with `typedef int BOOL` therefore
we need to use a `int32` on the stack.

* Docs/better homepage (#1591)

* Better carousel width

* Learn more button

* Learn more button

* Fix button/width

* Update introduction

* Fix outline colour

* remove custom css

* remove default css

* use secondary button

* Update sponsors. Improve website for mobile.

* v2.0.0-beta.39

* fix docs again

(cherry picked from commit 0a9ae0d)

* Fix docs for good

* Docs FTLOG

* Fix docs again!

* Fix Varly link

* Fix Varly link

* Update example for macOS Menu (#1600)

* Update example for macOS compatibility

Reference #1588

* Add comments, remove duplicates

* Add compatibility for Go1.17 (#1605)

* v2.0.0-beta.39.2

* [v2] Fix plain template (#1609)

* [template] Add missing main.go file to plain template

* [dev] If wails could not be find in go.mod output a gracefull error message

* Install dev dependencies before starting dev mode (#1615)

* Add Show() and Hide() to runtime to show/hide application (#1599)

* Add Show() and Hide() to runtime to show/hide application

* Fix devserver

* Update API docs

* Fix container size (#1616)

* Update Crowdin configuration file

* Update Crowdin configuration file

* Update Crowdin configuration file

* Bump terser from 4.8.0 to 4.8.1 in /runtime/js (#1625)

Bumps [terser](https://github.com/terser/terser) from 4.8.0 to 4.8.1.
- [Release notes](https://github.com/terser/terser/releases)
- [Changelog](https://github.com/terser/terser/blob/master/CHANGELOG.md)
- [Commits](https://github.com/terser/terser/commits)

---
updated-dependencies:
- dependency-name: terser
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump terser from 5.12.0 to 5.14.2 in /website (#1627)

Bumps [terser](https://github.com/terser/terser) from 5.12.0 to 5.14.2.
- [Release notes](https://github.com/terser/terser/releases)
- [Changelog](https://github.com/terser/terser/blob/master/CHANGELOG.md)
- [Commits](https://github.com/terser/terser/commits)

---
updated-dependencies:
- dependency-name: terser
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Co-authored-by: Antonio <acheong@student.dalat.org>
Co-authored-by: Lea Anthony <lea.anthony@gmail.com>
Co-authored-by: stffabi <stffabi@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting feedback More information is required from the requestor
Projects
None yet
Development

No branches or pull requests

3 participants