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

Support message, nameFieldLabel and showsTagField for macOS dialog #8556

Merged
merged 13 commits into from Feb 9, 2017

Conversation

Projects
None yet
2 participants
@yamgent
Contributor

yamgent commented Feb 1, 2017

Fixes #7783.

showSaveDialog() now supports additional attributes for setting the message, nameFieldLabel and showsTagField.

showOpenDialog() also supports message (nameFieldLabel and showsTagField doesn't seem to change the dialog box though).

  const {dialog} = require('electron');
  dialog.showSaveDialog({
    title: 'Save File',
    message: 'Select a location to save special file.',
    nameFieldLabel: 'Special Name',
    showsTagField: true
  });

screen shot 2017-02-02 at 12 05 05 am

@kevinsawicki

Looks pretty good, left a few minor comments, thanks for implementing this.

Show outdated Hide outdated lib/browser/api/dialog.js
Show outdated Hide outdated atom/browser/ui/file_dialog_mac.mm
Show outdated Hide outdated atom/browser/ui/file_dialog_mac.mm
Show outdated Hide outdated atom/browser/api/atom_api_dialog.cc
Show outdated Hide outdated lib/browser/api/dialog.js
if (message == null) {
message = ''
} else if (typeof message !== 'string') {
throw new TypeError('Message must be a string')

This comment has been minimized.

@kevinsawicki

kevinsawicki Feb 7, 2017

Contributor

Can you add an assertion for this error in spec/api-dialog-spec.js?

@kevinsawicki

kevinsawicki Feb 7, 2017

Contributor

Can you add an assertion for this error in spec/api-dialog-spec.js?

if (nameFieldLabel == null) {
nameFieldLabel = ''
} else if (typeof nameFieldLabel !== 'string') {
throw new TypeError('Name field label must be a string')

This comment has been minimized.

@kevinsawicki

kevinsawicki Feb 7, 2017

Contributor

Can you add an assertion for this error in spec/api-dialog-spec.js?

@kevinsawicki

kevinsawicki Feb 7, 2017

Contributor

Can you add an assertion for this error in spec/api-dialog-spec.js?

@yamgent

This comment has been minimized.

Show comment
Hide comment
@yamgent

yamgent Feb 9, 2017

Contributor

Thanks for your review! I will make the changes asap.

Contributor

yamgent commented Feb 9, 2017

Thanks for your review! I will make the changes asap.

yamgent added some commits Feb 1, 2017

🍎 Add additional options for Mac's save dialog
Support additional attributes available in macOS's NSSavePanel: message,
nameFieldLabel and showsTagField
🍎 Add additional options for Mac's open dialog
Support an additional attributes available in macOS's NSOpenPanel:
message.
Change the default value of showsTagField to true
The default value of showsTagField in macOS's NSSavePanel is true.

Therefore, in order to follow the standard behavior and not break
backwards-compatibility, let's change the default value of
showsTagField to true.

Reference:
https://developer.apple.com/reference/appkit/nssavepanel/1525589-showstagfield?language=objc
Change qualifier of ShowSaveDialog() parameter
The normal convention in the codebase is to not use references
or 'const' for primitives like 'bool' and 'int'.
@yamgent

This comment has been minimized.

Show comment
Hide comment
@yamgent

yamgent Feb 9, 2017

Contributor

@kevinsawicki I rebased on top of the latest master and incorporated all of your suggested changes.

Contributor

yamgent commented Feb 9, 2017

@kevinsawicki I rebased on top of the latest master and incorporated all of your suggested changes.

kevinsawicki added some commits Feb 9, 2017

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Feb 9, 2017

Contributor

Thanks for this @yamgent, I merged #8623 into this branch which added a DialogSettings helper object so the method signatures in file_dialog.h no longer needed to change when we add new options and options you added could just be added to that struct and then used directly in file_dialog.mm.

Great job on this 👍 🚀

Contributor

kevinsawicki commented Feb 9, 2017

Thanks for this @yamgent, I merged #8623 into this branch which added a DialogSettings helper object so the method signatures in file_dialog.h no longer needed to change when we add new options and options you added could just be added to that struct and then used directly in file_dialog.mm.

Great job on this 👍 🚀

@kevinsawicki kevinsawicki merged commit 5bf60ad into electron:master Feb 9, 2017

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment