[v2, options] Add EnableDefaultContextMenu option #2733

mmghv · 2023-06-19T05:11:39Z

Description

This PR adds the option EnableDefaultContextMenu which enables the browser's default context-menu in production. This menu is already enabled in development, as well as in debug builds and production builds with the -devtools flag.

Note: It's disabled by default so the default behavior is not changed.
Note: Enabling this option alone doesn't include the devtools Inspect menu.

Reference issues :
#2438
#1835

Screenshot

Type of change

New feature (non-breaking change which adds functionality)
This change requires a documentation update

How Has This Been Tested?

Windows
macOS
Linux

Here's how to test it :

1- Create a new project wails init -n testproject -t vanilla
2- Follow these instructions (replace [IDofThePR] with 2733), and make sure to add the replace directive in go.mod.
3- Change testproject/main.go to add this option :

func main() {
	// ...
	err := wails.Run(&options.App{
		// ...
		EnableDefaultContextMenu: false,
	})
	// ...
}

Expected behavior

In development and in wails build -debug and wails build -devtools, the context-menu should be available with the devtools inspector whether EnableDefaultContextMenu is true or false.
In normal production build wails build, the context-menu should ONLY be available when EnableDefaultContextMenu is true, and the devtools SHOULD NOT be present in the menu.

Test Configuration

wails doctor

# Wails
Version  | v2.5.1
Revision | fa851f29c53c81733407178f3a55d0129cfd51c3
Modified | true

# System
┌────────────────────────────────────┐
| OS           | Windows 10 Pro      |
| Version      | 2009 (Build: 19045) |
| ID           | 22H2                |
| Go Version   | go1.20.4            |
| Platform     | windows             |
| Architecture | amd64               |
└────────────────────────────────────┘

# Dependencies
┌───────────────────────────────────────────────────────┐
| Dependency | Package Name | Status    | Version       |
| WebView2   | N/A          | Installed | 114.0.1823.51 |
| Nodejs     | N/A          | Installed | 18.14.1       |
| npm        | N/A          | Installed | 9.3.1         |
| *upx       | N/A          | Installed | upx 4.0.2     |
| *nsis      | N/A          | Available |               |
└─────────────── * - Optional Dependency ───────────────┘

# Diagnosis
Optional package(s) installation details:
  - nsis : More info at https://wails.io/docs/guides/windows-installer/

 SUCCESS  Your system is ready for Wails development!

Checklist:

I have updated website/src/pages/changelog.mdx with details of this PR
My code follows the general coding style of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

mmghv · 2023-06-19T05:40:11Z

Tested on Linux, works as expected.

wails doctor

# Wails
Version         | v2.5.1
Revision        | 7fd413699cdb3f5ce7d15517cb57f7355c2f66da
Modified        | false
Package Manager | apt

# System
┌─────────────────────────┐
| OS           | Ubuntu   |
| Version      | 22.04    |
| ID           | ubuntu   |
| Go Version   | go1.20.5 |
| Platform     | linux    |
| Architecture | amd64    |
└─────────────────────────┘

# Dependencies
┌──────────────────────────────────────────────────────────────────────────┐
| Dependency | Package Name          | Status    | Version                 |
| *docker    | docker.io             | Installed |                         |
| gcc        | build-essential       | Installed | 11.3.0                  |
| libgtk-3   | libgtk-3-dev          | Installed | 3.24.33-1ubuntu2        |
| libwebkit  | libwebkit2gtk-4.0-dev | Installed | 2.38.6-0ubuntu0.22.04.1 |
| npm        | npm                   | Installed | 9.5.1                   |
| *nsis      | nsis                  | Available | 3.08-2                  |
| pkg-config | pkg-config            | Installed | 0.29.2-1ubuntu3         |
└──────────────────────── * - Optional Dependency ─────────────────────────┘

# Diagnosis
Optional package(s) installation details:
  - nsis: sudo apt install nsis

 SUCCESS  Your system is ready for Wails development!

@leaanthony Please test on macOS when you have the time.

leaanthony · 2023-06-19T09:03:01Z

Works fine on Mac. The thing to consider is that enabling the context menu on Mac enables all the context menus which includes "Reload" for the page.

mmghv · 2023-06-19T09:47:39Z

Yes, same on Windows & Linux, also there's a menu to save the website HTML, it would be a tradeoff, for me for most of the use cases I'll use Wails for, it's totally worth it to add the copy/paste menu which is essential IMO for inputs to the end-users, adding a custom copy/paste is somewhat hacky with JS, for critical apps you would disable it and add your custom solution.

Maybe in the future we could disable some of the menu items if that's something the webview supports, for now that's more than good enough for many use cases.

mmghv · 2023-06-19T09:53:18Z

Also a quick fix would be for the developer to disable the context menu for anything that is not an Input, it's pretty easy with JS, that way you wouldn't get (reload, back, download image, save (website) as, ...) menus, so it's actually a much better solution to do that than building your own menu.

EDIT
Here's JS code to do that, it also allows context menu on any selected (non-editable) text so you can copy it, this is the perfect solution for me, I get all the benefits of the native context menu without any of the "web" related menus I don't want:

document.oncontextmenu = function(e) {
  // Allow context menu on any input
  if (e.target && ['INPUT', 'TEXTAREA'].includes(e.target.tagName)) {
    return;
  }

  // Allow context menu on any text selection
  if (window.getSelection().toString()) {
    return;
  }

  // Disable the context menu otherwise
  e.preventDefault();
}

Tested on Windows & Linux

@leaanthony Ready for review.

leaanthony · 2023-06-19T10:29:32Z

Also a quick fix would be for the developer to disable the context menu for anything that is not an Input, it's pretty easy with JS, that way you wouldn't get (reload, save image, save website, ...) menus, so it's actually a much better solution to do that than building your own menu.

Did you try that out? I'm unsure if hooking into the context menu event will actually prevent the native one showing. If it does, then one strategy is to do something similar to how drag works: you could have an attribute like --default-contextmenu and have show and hide as valid values. That way you could have easy control over the default context menu in HTML.

mmghv · 2023-06-19T10:32:50Z

@leaanthony Yes I tested it on Windows & Linux, this event is usually used to disable the native context menus in the web, actually while I was doing this PR I found out that's how you disable it on one or two of the platforms.

That's great! I think having an html attribute to control this may be pretty cool then. If I'm right, this can be added after this PR with no breakages...

mmghv · 2023-06-19T10:42:27Z

That's great! I think having an html attribute to control this may be pretty cool then. If I'm right, this can be added after this PR with no breakages...

I think so, but having a broader solution that just works is easier for the developer, or do you mean adding the broader solution + HTML attributes for custom elements?

leaanthony · 2023-06-19T10:56:10Z

@mmghv - any chance you could do a go mod tidy and push the changes to see if it fixes the mac build issue? Thanks 🙏

leaanthony · 2023-06-19T10:57:05Z

I think so, but having a broader solution that just works is easier for the developer, or do you mean adding the broader solution + HTML attributes for custom elements?

Yeah, it's just something we add on top to this rather than having each dev doing JS hacks.

mmghv · 2023-06-19T10:57:09Z

I think you're right, adding the HTML attribute option would give the developer a fine control out of the box, while having the option to implement the broader solution himself, we could also put it in the documentation because I suspect most would just do that.

I'll work on a PR right after this one.

leaanthony · 2023-06-19T10:57:57Z

I'll work on a PR right after this one.

You're on 🔥

…ithub.com/mmghv/wails into feature/enable-default-contextmenu-option

mmghv · 2023-06-19T11:04:53Z

@mmghv - any chance you could do a go mod tidy and push the changes to see if it fixes the mac build issue? Thanks 🙏

Done, can you re-trigger the tests?

leaanthony · 2023-06-19T20:32:06Z

Nice one @mmghv 🙏

mmghv added 2 commits June 19, 2023 07:36

[v2, options] Add EnableDefaultContextMenu option

25b8e10

Update changelog & docs

7fd4136

mmghv marked this pull request as ready for review June 19, 2023 09:47

leaanthony approved these changes Jun 19, 2023

View reviewed changes

Merge branch 'master' into feature/enable-default-contextmenu-option

9100289

mmghv added 2 commits June 19, 2023 14:02

go mod tidy (./v2)

c2cc3f9

Merge branch 'feature/enable-default-contextmenu-option' of https://g…

69c2fa2

…ithub.com/mmghv/wails into feature/enable-default-contextmenu-option

leaanthony self-requested a review June 19, 2023 11:06

leaanthony approved these changes Jun 19, 2023

View reviewed changes

leaanthony merged commit 4162f09 into wailsapp:master Jun 19, 2023

mmghv deleted the feature/enable-default-contextmenu-option branch June 27, 2023 09:17

mmghv mentioned this pull request Jun 27, 2023

[v2] Add smart default context-menu functionality (backported from v3) #2748

Merged

13 tasks

mmghv mentioned this pull request Sep 14, 2023

Right click to open inspector does not work when build is generated with -devtools flag only #2908

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[v2, options] Add EnableDefaultContextMenu option #2733

[v2, options] Add EnableDefaultContextMenu option #2733

mmghv commented Jun 19, 2023 •

edited

Loading

mmghv commented Jun 19, 2023

leaanthony commented Jun 19, 2023

mmghv commented Jun 19, 2023

mmghv commented Jun 19, 2023 •

edited

Loading

leaanthony commented Jun 19, 2023 •

edited

Loading

mmghv commented Jun 19, 2023 •

edited by leaanthony

Loading

mmghv commented Jun 19, 2023

leaanthony commented Jun 19, 2023

leaanthony commented Jun 19, 2023

mmghv commented Jun 19, 2023

leaanthony commented Jun 19, 2023

mmghv commented Jun 19, 2023

leaanthony commented Jun 19, 2023

[v2, options] Add EnableDefaultContextMenu option #2733

[v2, options] Add EnableDefaultContextMenu option #2733

Conversation

mmghv commented Jun 19, 2023 • edited Loading

Description

Screenshot

Type of change

How Has This Been Tested?

Here's how to test it :

Expected behavior

Test Configuration

Checklist:

mmghv commented Jun 19, 2023

leaanthony commented Jun 19, 2023

mmghv commented Jun 19, 2023

mmghv commented Jun 19, 2023 • edited Loading

leaanthony commented Jun 19, 2023 • edited Loading

mmghv commented Jun 19, 2023 • edited by leaanthony Loading

mmghv commented Jun 19, 2023

leaanthony commented Jun 19, 2023

leaanthony commented Jun 19, 2023

mmghv commented Jun 19, 2023

leaanthony commented Jun 19, 2023

mmghv commented Jun 19, 2023

leaanthony commented Jun 19, 2023

mmghv commented Jun 19, 2023 •

edited

Loading

mmghv commented Jun 19, 2023 •

edited

Loading

leaanthony commented Jun 19, 2023 •

edited

Loading

mmghv commented Jun 19, 2023 •

edited by leaanthony

Loading