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

navigator: fix explorer 'open' command #8228

Merged
merged 1 commit into from
Aug 3, 2020
Merged

Conversation

vince-fugnitto
Copy link
Member

What it does

Fixes: #1308
Fixes: #2675

The following pull-request updates the open command present in the explorer context-menu to only be applicable for file resources and not directories (as directories cannot be opened). The context-menu is only visible and enabled if in the list of selectedNodes a file is present, and will only attempt to open files when executed.

How to test

The test-cases are performed using the explorer context-menu:

  • open is available when right-clicking a file node, executing the command opens the editor
  • open is available when right-clicking multiple selected file nodes, executing the command opens all the editors
  • open is not available when right-clicking a directory node
  • open is not available when right-clicking multiple selected directory nodes
  • open is available when selecting both a file node and directory node, executing the command opens only the editor(s) and no error is thrown

Review checklist

Reminder for reviewers

Signed-off-by: vince-fugnitto [email protected]

@vince-fugnitto vince-fugnitto added bug bugs found in the application navigator issues related to the navigator/explorer menus issues related to the menu labels Jul 23, 2020
@vince-fugnitto vince-fugnitto self-assigned this Jul 23, 2020
@vince-fugnitto vince-fugnitto force-pushed the vf/nav-open branch 2 times, most recently from a56cd77 to 8f1f00f Compare July 27, 2020 14:10
@akosyakov akosyakov requested a review from kittaakos July 28, 2020 08:05
Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is fine to me.

This PR means that it is not possible to add Open or Open With... for folder anymore. I don't expect downstream projects doing it, but checking with @eclipse-theia/ecd-theia-committers

packages/navigator/src/browser/navigator-contribution.ts Outdated Show resolved Hide resolved
the following commit fixes the `open` command for the navigator
to only display the command for file nodes and not directories (since
directories cannot be opened).

Signed-off-by: vince-fugnitto <[email protected]>
Copy link
Contributor

@Anasshahidd21 Anasshahidd21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I tested with all the use cases, and open command works only when it is mentioned to do so.

Tested on Windows.

@akosyakov
Copy link
Member

@vince-fugnitto merging?

@vince-fugnitto vince-fugnitto merged commit 8ac1e5f into master Aug 3, 2020
@vince-fugnitto vince-fugnitto deleted the vf/nav-open branch August 3, 2020 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs found in the application menus issues related to the menu navigator issues related to the navigator/explorer
Projects
None yet
3 participants