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

plugin: fix dialog-main implementation #9179

Merged
merged 1 commit into from
Mar 23, 2021
Merged

Conversation

vince-fugnitto
Copy link
Member

What it does

Fixes: #9177

The following commit updates the dialog extension API. The changes include updating the use of factories in order to use the
fileDialogService which already properly handles the dialog logic, and includes implementations in both the browser and electron.

The changes also include:

  • adding title as an available prop
  • adding extra handling when using both canSelectFiles and
    canSelectFolders simultaenously in the electron target.

How to test

Using the following plugin verify that the browser and electron applications correctly trigger their respective open and save dialogs through the commands:

  • Dialog: Open
  • Dialog: Save

Review checklist

Reminder for reviewers

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

@vince-fugnitto vince-fugnitto added electron issues related to the electron target vscode issues related to VSCode compatibility labels Mar 10, 2021
@vince-fugnitto vince-fugnitto self-assigned this Mar 10, 2021
@vince-fugnitto vince-fugnitto marked this pull request as ready for review March 19, 2021 16:42
Copy link
Contributor

@kittaakos kittaakos left a comment

Choose a reason for hiding this comment

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

The code looks good to me. I have verified it, and it worked.

Thank you so much for the test extension. It was super easy to verify the fix 👍

packages/plugin-ext/src/common/plugin-api-rpc.ts Outdated Show resolved Hide resolved
The following commit updates the `dialog` extension API.
The changes include updating the use of factories in order to use the
`fileDialogService` which already properly handles the dialog logic, and
includes implementations in both the `browser` and `electron`.

The changes also include:
- adding `title` as an available prop
- adding extra handling when using both `canSelectFiles` and
  `canSelectFolders` simultaenously in the electron target.

Signed-off-by: vince-fugnitto <[email protected]>
@vince-fugnitto
Copy link
Member Author

@kittaakos thank you for taking the time to review, I generalized the documentation a bit :)

Copy link
Contributor

@RomanNikitenko RomanNikitenko left a comment

Choose a reason for hiding this comment

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

I tested on Linux for electron and browser assemblies.

I see the following differences against the master branch:

  • for the electron assembly the system dialog is displayed for the both commands
  • for the electron assembly the system allows to select only folders (not files) when I use the Dialog: Open command
  • the custom title is displayed for a dialog

I see the same behavior for VS Code. It works well for me!

@vince-fugnitto vince-fugnitto merged commit eb119cb into master Mar 23, 2021
@vince-fugnitto vince-fugnitto deleted the vf/plugin-dialog branch March 23, 2021 23:23
@github-actions github-actions bot added this to the 1.12.0 milestone Mar 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
electron issues related to the electron target vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

electron: plugin system incorrectly opens browser dialog
3 participants