Prevent default browser context-menu #6430

vince-fugnitto · 2019-10-21T18:19:55Z

What it does

Fixes #4007

Current Behavior

When right-clicking certain content (ex: the application without any editors opened) the default browser menu is displayed. This breaks up the user experience and can become quite annoying.

PR

The PR adds additional handling to the window contextmenu event to prevent the default browser behavior. Existing context menus such as for an editor, or widgets remain unchanged.

How to test

Verify Default Behavior

open the application
ensure that no editors are opened
attempt to right-click in the empty area (nothing should happen) - previously the browser context menu was displayed

Verify Regression

open the application
open an editor
right-click within the editor, the editor context menu should be visible
do the same as step 3 with a widget tab

Review checklist

as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

as a reviewer, I agree to behave in accordance with the review guidelines

Signed-off-by: Vincent Fugnitto vincent.fugnitto@ericsson.com

Fixes #4007 Add additional handling to handle `contexmenu` events which would display the browsers default contextmenu which breaks up the user experience. The default browser `contextmenu` is thus disabled. Signed-off-by: Vincent Fugnitto <vincent.fugnitto@ericsson.com>

elaihau · 2019-10-22T03:54:43Z

Tested in my local env.

I confirm:

no context menu was populated when right-clicking the empty area, if no editors were open
context menu was populated when right-clicking the editor that is opened in the main, the right, the left, and the bottom panel,
no context menu was populated when right-clicking the empty main panel, if the editor was docked in the right, the left, and the bottom panel.

elaihau

looks good to me, thank you !

elaihau · 2019-10-22T04:10:48Z

I found #6431 when testing but I believe it is a separated issue.

AndrienkoAleksandr · 2019-10-22T07:40:06Z

Hello @vince-fugnitto , I tested your pr and I see that after merge this one we are losing context menu for terminal. Maybe we should create separated issue to create context menu for terminal.

vince-fugnitto · 2019-10-22T10:17:14Z

@AndrienkoAleksandr I believe the proper fix for the terminal would be to create it's own context menu in which we can add new items. See #6216.

I can hold off on this PR if you'd like until the context menu for the terminal is created.
Which current functionality do you use in the terminal default browser context-menu?

akosyakov · 2019-10-23T04:42:51Z

@AndrienkoAleksandr Could you elaborate why browser context menu is helpful for terminals till #6216 is fixed?

AndrienkoAleksandr · 2019-10-23T06:51:49Z

Hello. I don't block merge this pr. I only applied information. Currently terminal context menu is useful only to copy and paste text, but we have shortcuts to do that. And yes, we need implement context menu. By the way Do we have an issue for that? And when we will implement context menu paste will be working only with electron. For browser version paste won't work for browser security reason.

kittaakos · 2019-10-23T07:50:18Z

@vince-fugnitto, please make sure extension developers have an easy way to re-enable the context menus if they want to.

kittaakos · 2019-10-23T09:38:12Z

I have just noticed, the context menu is not available anymore from input and textarea. It should work. For instance, I should be able to cut something via the context menu from the SIW view's input and paste it also via the context menu to the SCM view's textarea.

We can do something similar the VS Code guys did, and enable only certain actions: electron/electron#4068 (Edit: I know the referenced issue is a bit off, I added to show that VS Code has support for the context menu in input and textare, so we cannot disable it either.)

lmcbout · 2019-10-23T11:40:48Z

Tested on Ubuntu 16.04 and Chrome

While testing this PR, I found the following:
Terminal | Problem view: context menu for "Toggle Maximized CTRL +M" is missing (Context menu when selecting the terminal title bar.
If you see the item in the context menu, just select the "explorer" view button to hide + show again the "explorer" view, then try to open the context menu over the terminal view (First occurrence only)

Search View: We should have a context menu to "Copy/Paste" as minimal. I know we can use keyboard shortcut to copy/paste, but I would also prefer to have a minimal context menu for some commands .

Explorer view: When selecting the context menu over the workspace name, there is no context menu to show the following views:
Npm scripts
Json Outline
FTP Explorere
File Explorer
Test View

vince-fugnitto · 2019-10-23T12:23:37Z

This occurs in master as well.

Search View: We should have a context menu to "Copy/Paste" as minimal. I know we can use keyboard shortcut to copy/paste, but I would also prefer to have a minimal context menu for some commands .

This is not in the scope of the PR. I am not adding context menus for widgets.

Explorer view: When selecting the context menu over the workspace name, there is no context menu to show the following views:
Npm scripts
Json Outline
FTP Explorere
File Explorer
Test View

You likely do not have them because you do not have the additional VS Code extensions provided.
They are not there by default in the repository.

lmcbout

Item 1: context menu for "Toggle Maximized CTRL +M" is missing
OK, I will open an issue about it

Item 2:
Search View: We should have a context menu to "Copy/Paste" as minimal. I know we can use >>keyboard shortcut to copy/paste, but I would also prefer to have a minimal context menu for some >>commands .

This is not in the scope of the PR. I am not adding context menus for widgets.

By removing the context menu, you also remove the item for the copy/paste which was available on the old context menu. If the context menu is not provided with this PR, you can open a new issue to add it later, I am fine with it.

Item 3: my mistake

vince-fugnitto · 2019-11-20T14:22:14Z

I have just noticed, the context menu is not available anymore from input and textarea. It should work. For instance, I should be able to cut something via the context menu from the SIW view's input and paste it also via the context menu to the SCM view's textarea.

We can do something similar the VS Code guys did, and enable only certain actions: electron/electron#4068 (Edit: I know the referenced issue is a bit off, I added to show that VS Code has support for the context menu in input and textare, so we cannot disable it either.)

I don't have any real urgency in merging the PR, I'm concerned about the issues you described about how users don't have the contextmenu in input and textarea fields. I believe the consensus is before merging the PR, to address the context menus in those areas?

vince-fugnitto · 2020-04-27T15:24:02Z

I'll put the pull-request on hold until we complete the following:

default context menus for 'inputs' and 'textarea'
default context menu for terminals: terminal: add context-menu #7632

vince-fugnitto added the ui/ux issues related to user interface / user experience label Oct 21, 2019

vince-fugnitto requested review from lmcbout and elaihau October 21, 2019 18:19

elaihau approved these changes Oct 22, 2019

View reviewed changes

akosyakov requested review from AndrienkoAleksandr and kittaakos October 23, 2019 07:53

kittaakos mentioned this pull request Oct 23, 2019

[electron] Support context menu for input and textarea #6438

Open

lmcbout approved these changes Oct 23, 2019

View reviewed changes

AndrienkoAleksandr approved these changes Oct 23, 2019

View reviewed changes

kittaakos mentioned this pull request Apr 2, 2020

GH-6428: Refactored the built-in monaco commands. #7481

Merged

2 tasks

vince-fugnitto closed this Apr 27, 2020

vince-fugnitto mentioned this pull request Jun 3, 2020

GH-7767: Added context menu for input and textArea #7943

Merged

1 task

vince-fugnitto deleted the vf/browser-context-menu branch September 29, 2022 12:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent default browser context-menu #6430

Prevent default browser context-menu #6430

vince-fugnitto commented Oct 21, 2019

elaihau commented Oct 22, 2019

elaihau left a comment

elaihau commented Oct 22, 2019

AndrienkoAleksandr commented Oct 22, 2019

vince-fugnitto commented Oct 22, 2019

akosyakov commented Oct 23, 2019

AndrienkoAleksandr commented Oct 23, 2019

kittaakos commented Oct 23, 2019

kittaakos commented Oct 23, 2019 •

edited

Loading

lmcbout commented Oct 23, 2019

vince-fugnitto commented Oct 23, 2019

lmcbout left a comment

vince-fugnitto commented Nov 20, 2019

vince-fugnitto commented Apr 27, 2020

Prevent default browser context-menu #6430

Prevent default browser context-menu #6430

Conversation

vince-fugnitto commented Oct 21, 2019

What it does

How to test

Review checklist

Reminder for reviewers

elaihau commented Oct 22, 2019

elaihau left a comment

Choose a reason for hiding this comment

elaihau commented Oct 22, 2019

AndrienkoAleksandr commented Oct 22, 2019

vince-fugnitto commented Oct 22, 2019

akosyakov commented Oct 23, 2019

AndrienkoAleksandr commented Oct 23, 2019

kittaakos commented Oct 23, 2019

kittaakos commented Oct 23, 2019 • edited Loading

lmcbout commented Oct 23, 2019

vince-fugnitto commented Oct 23, 2019

lmcbout left a comment

Choose a reason for hiding this comment

vince-fugnitto commented Nov 20, 2019

vince-fugnitto commented Apr 27, 2020

kittaakos commented Oct 23, 2019 •

edited

Loading