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

feat: promisify dialog.showSaveDialog() #17054

Merged
merged 4 commits into from Mar 5, 2019

Conversation

@codebytere
Copy link
Member

commented Feb 19, 2019

Description of Change

Promisifies dialog.showSaveDialog().

cc @ckerr @MarshallOfSound @deepak1556

Checklist

Release Notes

Notes: Split dialog.showSaveDialog() into a synchronous version and a version that returns a Promise

@codebytere codebytere requested a review from as a code owner Feb 19, 2019

@codebytere

This comment has been minimized.

Copy link
Member Author

commented Feb 19, 2019

Needs rebase on #17050 once it's merged.

@codebytere codebytere force-pushed the promisify-showsavedialog branch 2 times, most recently from 3f7d228 to c9dba0b Feb 20, 2019

@codebytere codebytere requested a review from as a code owner Feb 20, 2019

@codebytere codebytere changed the title [wip] feat: promisify dialog.showSaveDialog() feat: promisify dialog.showSaveDialog() Feb 20, 2019

@codebytere codebytere force-pushed the promisify-showsavedialog branch from ef2d87c to 088fae3 Feb 25, 2019

@zcbenz

This comment has been minimized.

Copy link
Member

commented Feb 25, 2019

Review in #16973 (review) also applies here.

@codebytere codebytere force-pushed the promisify-showsavedialog branch 2 times, most recently from f4ac326 to 8c9d837 Feb 27, 2019

@codebytere codebytere force-pushed the promisify-showsavedialog branch 6 times, most recently from e2855cd to 063fd5e Mar 1, 2019

Show resolved Hide resolved lib/browser/api/dialog.js Outdated
Show resolved Hide resolved docs/api/dialog.md Outdated
Show resolved Hide resolved docs/api/dialog.md
Show resolved Hide resolved atom/browser/ui/file_dialog_mac.mm Outdated
Show resolved Hide resolved docs/api/dialog.md Outdated
Show resolved Hide resolved lib/browser/api/dialog.js

@codebytere codebytere force-pushed the promisify-showsavedialog branch from fa9b40b to fe0fa2d Mar 5, 2019

@zcbenz

zcbenz approved these changes Mar 5, 2019

Show resolved Hide resolved atom/browser/ui/file_dialog_win.cc Outdated
Show resolved Hide resolved atom/browser/ui/file_dialog_mac.mm Outdated
Show resolved Hide resolved atom/browser/ui/file_dialog_mac.mm Outdated
Show resolved Hide resolved atom/browser/ui/file_dialog_gtk.cc Outdated
Show resolved Hide resolved atom/browser/ui/file_dialog_gtk.cc Outdated

codebytere added some commits Feb 27, 2019

@codebytere codebytere force-pushed the promisify-showsavedialog branch from 7960f88 to c2c9e68 Mar 5, 2019

Show resolved Hide resolved atom/browser/ui/file_dialog_win.cc Outdated
@miniak

miniak approved these changes Mar 5, 2019

@codebytere codebytere merged commit 6cb7b8d into master Mar 5, 2019

14 of 15 checks passed

Artifact Comparison Changes Detected
Details
Semantic Pull Request ready to be squashed
Details
WIP Ready for review
Details
appveyor: win-ia32-debug AppVeyor build succeeded
Details
appveyor: win-ia32-testing AppVeyor build succeeded
Details
appveyor: win-ia32-testing-pr AppVeyor build succeeded
Details
appveyor: win-x64-debug AppVeyor build succeeded
Details
appveyor: win-x64-testing AppVeyor build succeeded
Details
appveyor: win-x64-testing-pr AppVeyor build succeeded
Details
build-linux Workflow: build-linux
Details
build-mac Workflow: build-mac
Details
electron-arm-testing Build #20190305.12 succeeded
Details
electron-arm64-testing Build #20190305.12 succeeded
Details
lint Workflow: lint
Details
release-notes Release notes found
@release-clerk

This comment has been minimized.

Copy link

commented Mar 5, 2019

Release Notes Persisted

Split dialog.showSaveDialog() into a synchronous version and a version that returns a Promise

@codebytere codebytere deleted the promisify-showsavedialog branch Mar 5, 2019

Kiku-Reise added a commit to Kiku-Reise/electron that referenced this pull request May 16, 2019

feat: promisify dialog.showSaveDialog() (electron#17054)
* feat: promisify dialog.showSaveDialog()

* address some feedback from review

* filename => filePath

* fix last filename => filePath
@cristiammercado

This comment has been minimized.

Copy link

commented May 18, 2019

On Windows (I don't know in other platforms), the variable canceled is returning the inverted value: when the save window is canceled (click on cancel button), it sets to false, and when the save button is clicked (a filanme is setted), it returns true.

I'm not if report this issue in a new bug @miniak @codebytere , I don't have a repo to test the issue but I printed in my project in console:

When I select a filename and click Save:

Screenshot_1

When I click on Cancel and don't select any filename:

Screenshot_2

@codebytere

This comment has been minimized.

Copy link
Member Author

commented May 18, 2019

I'll handle that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.