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

Add support for checkbox with dialog.showMessageBox #8590

Merged
merged 2 commits into from Feb 9, 2017

Conversation

Projects
None yet
3 participants
@poiru
Member

poiru commented Feb 5, 2017

This adds the checkboxLabel and checkboxChecked options to display a
checkbox in the message box. Fixes #6048.

Example screenshots:

mac

linux

windows

@kevinsawicki

Looks great, left a few minor comments.

Show outdated Hide outdated atom/browser/ui/message_box_gtk.cc
Show outdated Hide outdated docs/api/dialog.md
@poiru

This comment has been minimized.

Show comment
Hide comment
@poiru

poiru Feb 7, 2017

Member

@kevinsawicki Maybe we should rename checkboxTitle to checkboxLabel for consistency with #8556. WDYT?

Member

poiru commented Feb 7, 2017

@kevinsawicki Maybe we should rename checkboxTitle to checkboxLabel for consistency with #8556. WDYT?

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Feb 7, 2017

Contributor

Maybe we should rename checkboxTitle to checkboxLabel for consistency with #8556. WDYT?

Sounds good to me 👍

Contributor

kevinsawicki commented Feb 7, 2017

Maybe we should rename checkboxTitle to checkboxLabel for consistency with #8556. WDYT?

Sounds good to me 👍

@poiru

This comment has been minimized.

Show comment
Hide comment
@poiru
Member

poiru commented Feb 8, 2017

poiru and others added some commits Feb 6, 2017

Add support for checkbox with dialog.showMessageBox
This adds the `checkboxLabel` and `checkboxChecked` options to display a
checkbox in the message box. Fixes #6048.
@@ -154,13 +169,16 @@ class GtkMessageBox : public NativeWindowObserver {
}
CHROMEGTK_CALLBACK_1(GtkMessageBox, void, OnResponseDialog, int);
CHROMEGTK_CALLBACK_0(GtkMessageBox, void, OnCheckboxToggled);

This comment has been minimized.

@kevinsawicki

kevinsawicki Feb 9, 2017

Contributor

@poiru I tested this out on Ubuntu 16.04 and initially saw this crash when CHROMEGTK_CALLBACK_1 was being used:

Received signal 11 SEGV_ACCERR 000000cf5b00
#0 0x7fd99563fd07 <unknown>
#1 0x7fd98c02e390 <unknown>
#2 0x000000cf5d90 atom::(anonymous namespace)::GtkMessageBox::OnCheckboxToggled()
#3 0x000000cf5b20 atom::(anonymous namespace)::GtkMessageBox::OnCheckboxToggledThunk()
#4 0x7fd98a387fa5 g_closure_invoke
#5 0x7fd98a399fc1 <unknown>
#6 0x7fd98a3a2d5c g_signal_emit_valist
#7 0x7fd98a3a308f g_signal_emit
#8 0x7fd98bbc7553 <unknown>
#9 0x7fd98a3881d4 <unknown>
#10 0x7fd98a3a29a6 g_signal_emit_valist
#11 0x7fd98a3a308f g_signal_emit
#12 0x000000cf4cd4 atom::(anonymous namespace)::GtkMessageBox::GtkMessageBox()

Switching it to a CHROMEGTK_CALLBACK_0 instead of CHROMEGTK_CALLBACK_1 made it go away and provided the right checkbox value. It looked like the gpointer was unused so it seemed safe to ignore.

@kevinsawicki

kevinsawicki Feb 9, 2017

Contributor

@poiru I tested this out on Ubuntu 16.04 and initially saw this crash when CHROMEGTK_CALLBACK_1 was being used:

Received signal 11 SEGV_ACCERR 000000cf5b00
#0 0x7fd99563fd07 <unknown>
#1 0x7fd98c02e390 <unknown>
#2 0x000000cf5d90 atom::(anonymous namespace)::GtkMessageBox::OnCheckboxToggled()
#3 0x000000cf5b20 atom::(anonymous namespace)::GtkMessageBox::OnCheckboxToggledThunk()
#4 0x7fd98a387fa5 g_closure_invoke
#5 0x7fd98a399fc1 <unknown>
#6 0x7fd98a3a2d5c g_signal_emit_valist
#7 0x7fd98a3a308f g_signal_emit
#8 0x7fd98bbc7553 <unknown>
#9 0x7fd98a3881d4 <unknown>
#10 0x7fd98a3a29a6 g_signal_emit_valist
#11 0x7fd98a3a308f g_signal_emit
#12 0x000000cf4cd4 atom::(anonymous namespace)::GtkMessageBox::GtkMessageBox()

Switching it to a CHROMEGTK_CALLBACK_0 instead of CHROMEGTK_CALLBACK_1 made it go away and provided the right checkbox value. It looked like the gpointer was unused so it seemed safe to ignore.

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Feb 9, 2017

Contributor

Awesome work on this @poiru, this will be useful to so many Electron apps 👍

Tested it on macOS 10.12, Windows 10, and Ubuntu 16.04, everything looked great 🚢 :shipit:

Contributor

kevinsawicki commented Feb 9, 2017

Awesome work on this @poiru, this will be useful to so many Electron apps 👍

Tested it on macOS 10.12, Windows 10, and Ubuntu 16.04, everything looked great 🚢 :shipit:

@kevinsawicki kevinsawicki merged commit e741097 into master Feb 9, 2017

8 of 9 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
electron-linux-arm Build #5458860 succeeded in 98s
Details
electron-linux-ia32 Build #5458861 succeeded in 90s
Details
electron-linux-x64 Build #5458862 succeeded in 164s
Details
electron-mas-x64 Build #3374 succeeded in 7 min 37 sec
Details
electron-osx-x64 Build #3383 succeeded in 8 min 5 sec
Details
electron-win-ia32 Build #2387 succeeded in 8 min 6 sec
Details
electron-win-x64 Build #2371 succeeded in 7 min 56 sec
Details

@kevinsawicki kevinsawicki deleted the showmessagebox-checkbox branch Feb 9, 2017

jviotti pushed a commit to resin-io/etcher that referenced this pull request Apr 25, 2017

Juan Cruz Viotti
upgrade: `electron` to v1.6.6
Some changes that we're particularly interested in:

- electron/electron#8590
- electron/electron#8852
- electron/electron#7631

Note that the `electron-prebuilt` packaged has been renamed to
`electron`.

Change-Type: patch
Changelog-Entry: Upgrade Electron to v1.6.6.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>

jviotti pushed a commit to resin-io/etcher that referenced this pull request Apr 25, 2017

Juan Cruz Viotti
upgrade: `electron` to v1.6.6 (#1357)
Some changes that we're particularly interested in:

- electron/electron#8590
- electron/electron#8852
- electron/electron#7631

Note that the `electron-prebuilt` packaged has been renamed to
`electron`.

Change-Type: patch
Changelog-Entry: Upgrade Electron to v1.6.6.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>

jviotti pushed a commit to resin-io/etcher that referenced this pull request Apr 25, 2017

Juan Cruz Viotti
refactor(GUI): turn the update notifier modal into a native dialog
Electron v1.6.1 introduced checkbox support to the native message
dialog, giving us everything that was needed to implement the update
notifier modal using a native dialog.

This change allows us to get rid of a lot code.

See: electron/electron#8590
Change-Type: patch
Changelog-Entry: Turn the update notifier modal into a native dialog.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>

jviotti pushed a commit to resin-io/etcher that referenced this pull request Apr 25, 2017

Juan Cruz Viotti
refactor(GUI): turn the update notifier modal into a native dialog
Electron v1.6.1 introduced checkbox support to the native message
dialog, giving us everything that was needed to implement the update
notifier modal using a native dialog.

This change allows us to get rid of a lot code.

See: electron/electron#8590
Change-Type: patch
Changelog-Entry: Turn the update notifier modal into a native dialog.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>

jviotti pushed a commit to resin-io/etcher that referenced this pull request Apr 27, 2017

Juan Cruz Viotti
refactor(GUI): turn the update notifier modal into a native dialog
Electron v1.6.1 introduced checkbox support to the native message
dialog, giving us everything that was needed to implement the update
notifier modal using a native dialog.

This change allows us to get rid of a lot code.

See: electron/electron#8590
Change-Type: patch
Changelog-Entry: Turn the update notifier modal into a native dialog.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>

jviotti pushed a commit to resin-io/etcher that referenced this pull request Apr 27, 2017

Juan Cruz Viotti
refactor(GUI): turn the update notifier modal into a native dialog
Electron v1.6.1 introduced checkbox support to the native message
dialog, giving us everything that was needed to implement the update
notifier modal using a native dialog.

This change allows us to get rid of a lot code.

See: electron/electron#8590
Change-Type: patch
Changelog-Entry: Turn the update notifier modal into a native dialog.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>

jviotti pushed a commit to resin-io/etcher that referenced this pull request May 2, 2017

Juan Cruz Viotti
refactor(GUI): turn the update notifier modal into a native dialog
Electron v1.6.1 introduced checkbox support to the native message
dialog, giving us everything that was needed to implement the update
notifier modal using a native dialog.

This change allows us to get rid of a lot code.

See: electron/electron#8590
Change-Type: patch
Changelog-Entry: Turn the update notifier modal into a native dialog.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>

jviotti pushed a commit to resin-io/etcher that referenced this pull request May 3, 2017

Juan Cruz Viotti
refactor(GUI): turn the update notifier modal into a native dialog
Electron v1.6.1 introduced checkbox support to the native message
dialog, giving us everything that was needed to implement the update
notifier modal using a native dialog.

This change allows us to get rid of a lot code.

See: electron/electron#8590
Change-Type: patch
Changelog-Entry: Turn the update notifier modal into a native dialog.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>

jviotti pushed a commit to resin-io/etcher that referenced this pull request May 3, 2017

Juan Cruz Viotti
refactor(GUI): turn the update notifier modal into a native dialog
Electron v1.6.1 introduced checkbox support to the native message
dialog, giving us everything that was needed to implement the update
notifier modal using a native dialog.

This change allows us to get rid of a lot code.

See: electron/electron#8590
Change-Type: patch
Changelog-Entry: Turn the update notifier modal into a native dialog.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>

jviotti pushed a commit to resin-io/etcher that referenced this pull request May 4, 2017

Juan Cruz Viotti
refactor(GUI): turn the update notifier modal into a native dialog
Electron v1.6.1 introduced checkbox support to the native message
dialog, giving us everything that was needed to implement the update
notifier modal using a native dialog.

This change allows us to get rid of a lot code.

See: electron/electron#8590
Change-Type: patch
Changelog-Entry: Turn the update notifier modal into a native dialog.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>

jviotti pushed a commit to resin-io/etcher that referenced this pull request May 4, 2017

Juan Cruz Viotti
refactor(GUI): turn the update notifier modal into a native dialog
Electron v1.6.1 introduced checkbox support to the native message
dialog, giving us everything that was needed to implement the update
notifier modal using a native dialog.

This change allows us to get rid of a lot code.

See: electron/electron#8590
Change-Type: patch
Changelog-Entry: Turn the update notifier modal into a native dialog.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>

jviotti pushed a commit to resin-io/etcher that referenced this pull request May 4, 2017

Juan Cruz Viotti
refactor(GUI): turn the update notifier modal into a native dialog
Electron v1.6.1 introduced checkbox support to the native message
dialog, giving us everything that was needed to implement the update
notifier modal using a native dialog.

This change allows us to get rid of a lot code.

See: electron/electron#8590
Change-Type: patch
Changelog-Entry: Turn the update notifier modal into a native dialog.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>

jviotti pushed a commit to resin-io/etcher that referenced this pull request May 7, 2017

Juan Cruz Viotti
refactor(GUI): turn the update notifier modal into a native dialog
Electron v1.6.1 introduced checkbox support to the native message
dialog, giving us everything that was needed to implement the update
notifier modal using a native dialog.

This change allows us to get rid of a lot code.

See: electron/electron#8590
Change-Type: patch
Changelog-Entry: Turn the update notifier modal into a native dialog.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>

jviotti pushed a commit to resin-io/etcher that referenced this pull request May 7, 2017

Juan Cruz Viotti
refactor(GUI): turn the update notifier modal into a native dialog
Electron v1.6.1 introduced checkbox support to the native message
dialog, giving us everything that was needed to implement the update
notifier modal using a native dialog.

This change allows us to get rid of a lot code.

See: electron/electron#8590
Change-Type: patch
Changelog-Entry: Turn the update notifier modal into a native dialog.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>

jviotti pushed a commit to resin-io/etcher that referenced this pull request May 15, 2017

Juan Cruz Viotti
refactor(GUI): turn the update notifier modal into a native dialog
Electron v1.6.1 introduced checkbox support to the native message
dialog, giving us everything that was needed to implement the update
notifier modal using a native dialog.

This change allows us to get rid of a lot code.

See: electron/electron#8590
Change-Type: patch
Changelog-Entry: Turn the update notifier modal into a native dialog.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>

jviotti pushed a commit to resin-io/etcher that referenced this pull request May 15, 2017

Juan Cruz Viotti
refactor(GUI): turn the update notifier modal into a native dialog
Electron v1.6.1 introduced checkbox support to the native message
dialog, giving us everything that was needed to implement the update
notifier modal using a native dialog.

This change allows us to get rid of a lot code.

See: electron/electron#8590
Change-Type: patch
Changelog-Entry: Turn the update notifier modal into a native dialog.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>

jviotti pushed a commit to resin-io/etcher that referenced this pull request May 22, 2017

Juan Cruz Viotti
refactor(GUI): turn the update notifier modal into a native dialog
Electron v1.6.1 introduced checkbox support to the native message
dialog, giving us everything that was needed to implement the update
notifier modal using a native dialog.

This change allows us to get rid of a lot code.

See: electron/electron#8590
Change-Type: patch
Changelog-Entry: Turn the update notifier modal into a native dialog.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>

jviotti pushed a commit to resin-io/etcher that referenced this pull request May 23, 2017

Juan Cruz Viotti
refactor(GUI): turn the update notifier modal into a native dialog (#…
…1359)

Electron v1.6.1 introduced checkbox support to the native message
dialog, giving us everything that was needed to implement the update
notifier modal using a native dialog.

This change allows us to get rid of a lot code.

See: electron/electron#8590
Change-Type: patch
Changelog-Entry: Turn the update notifier modal into a native dialog.
Signed-off-by: Juan Cruz Viotti <jviotti@openmailbox.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment