Disallow duplicate windows. #6594

pull casey wants to merge 1 commits into bitcoin:master from casey:duplicate-windows changing 4 files +21 −11
  1. casey commented at 7:58 PM on August 27, 2015: contributor

    Fixes #3569.

    This change disallows the creation of multiple instances of the command line options, used send addresses, and used receive addresses windows.

  2. laanwj added the label GUI on Aug 28, 2015
  3. jonasschnelli commented at 2:46 PM on August 28, 2015: contributor

    Nice! Tested a bit.

    Two points:

    • why is the "Sign message" dialog is modal and the "Sending Addresses" and "Receiving Addresses" not? (indirectly related to this PR)
    • This PR works, but it would be nice, if the window would get the focus when a user selects "Sending Addresses (or "Receiving Addresses") if the window is already open in background.
  4. laanwj commented at 2:55 PM on August 31, 2015: member

    Concept ack.

  5. casey commented at 5:22 PM on August 31, 2015: contributor

    I tried adding calls to widget->activateWindow() and widget->raise(), but my weirdo window manager (xmonad) isn't respecting them so I can't test. @jonasschnelli, can you pull casey:duplicate-windows-foreground and see if that fixes it?

    As far as modal vs non-modal, I can't really come up with a compelling argument for it either way. Do users leave up the sign/verify dialogs while they do other thing?

  6. jonasschnelli commented at 7:56 AM on September 1, 2015: contributor

    @casey I also don't have an strong opinion on modal/non-modal. Just tested on Ubuntu and OSX and the focus issue is still unsolved. Could you not just call activateWindow() (http://doc.qt.io/qt-4.8/qwidget.html#activateWindow). IIRC I once have implemented that also in https://github.com/bitcoin/bitcoin/blob/master/src/qt/macdockiconhandler.mm#L129.

  7. casey commented at 7:59 PM on September 1, 2015: contributor

    @jonasschnelli Which version of ubuntu and which desktop environment are you testing with? When I test with ubuntu vivid with unity, I can't actually get the main window to the foreground when the "sending addresses" or "receiving addresses" windows are up, they stay in the foreground, although I can focus the main window.

  8. jonasschnelli commented at 8:57 AM on September 2, 2015: contributor

    @casey: Right. On Ubuntu the "sending addresses" or "receiving addresses" windows are always in front. On OSX (and probably also on Windows), the window does not keep in front always. There it would be good if the window gets the focus if one selects "sending addresses" or "receiving addresses" again from the menu. On Ubuntu focusing the window (even if it is always in front but un-focused) would also be good.

  9. casey commented at 3:56 PM on September 3, 2015: contributor

    @jonasschnelli I still can't get the windows to come to the foreground on all platforms; even with calls to activeateWindow(), behavior is still inconsistent.

    What about just making all three dialogues modal, so that they're just always in the foreground, and users have to dismiss them to continue? This would make everything consistent.

  10. MarcoFalke commented at 4:20 PM on September 3, 2015: member

    I think the "* addresses"-windows are non-modal so the user can easily copy more than one address into the "Send"-tab. But then there is also the "Choose previously used address"-button in the "Send"-tab providing an alternative to copy-paste sending addresses... So, making all three dialogues modal sounds ok to me.

  11. laanwj commented at 4:31 PM on September 3, 2015: member

    I personally prefer being able to open multiple windows at a time (not of the same type obviously - I agree with this pull). Modal dialogs are meant for when some action has to be completed before another, but if there is no sequential control flow, there is no reason to not allow interacting with them at the same time.

  12. casey commented at 7:48 PM on September 3, 2015: contributor

    In that case I think we should merge the PR as is. I haven't been able to get the right behavior in a cross platform way, no matter how many calls to activateWindow() and raise() that I insert :-P

  13. jonasschnelli commented at 7:00 AM on September 4, 2015: contributor

    Tested ACK on merging this as it is. The non-focus while reselecting over the menu is a nitpick.

  14. fanquake commented at 7:28 AM on September 4, 2015: member

    utACK

    On Friday, September 4, 2015, Jonas Schnelli notifications@github.com wrote:

    ACK on merging this as it is. The non-focus while reselecting over the menu is a nitpick.

    — Reply to this email directly or view it on GitHub #6594 (comment).

  15. jonasschnelli commented at 7:42 AM on September 4, 2015: contributor

    @casey: This commit (https://github.com/jonasschnelli/bitcoin/commit/a272c804ad04346ab86af4eb9cc64967b2c54e95) would fix the focus issue. Mind cherry pick this commit?

  16. laanwj commented at 2:48 PM on September 4, 2015: member

    This shouldn't include the commit "rpc-tests: re-enable rpc-tests for Windows". (nor the two commits before that)

  17. Disallow duplicate windows. 5ffaaba3a1
  18. MarcoFalke commented at 8:14 AM on September 5, 2015: member

    Tested ACK (5ffaaba, Fedora) @jonasschnelli If you are happy with 5ffaaba maybe you could also create a windows build at https://builds.jonasschnelli.ch/pulls/6594/ ?

  19. jonasschnelli commented at 6:10 PM on September 5, 2015: contributor

    @MarcoFalke: just triggered a gitian build of this PR: https://builds.jonasschnelli.ch/pulls/6594/

  20. MarcoFalke commented at 11:37 AM on September 6, 2015: member

    Great, thanks for the build!

    Tested ACK (Win 7)

  21. jonasschnelli commented at 12:04 PM on September 7, 2015: contributor

    Tested ACK.

  22. btcdrak commented at 11:34 PM on September 7, 2015: contributor

    @casey just a little github trick, if a PR fixed another issue, you can put "fixes #3569" in your commit message and it will close the ticket when this gets the PR gets merged.

  23. fanquake commented at 11:38 PM on September 7, 2015: member

    @btcdrak I'm pretty sure that also works with when you have it in the PR description.

  24. btcdrak commented at 11:42 PM on September 7, 2015: contributor

    @fanquake Maybe, they are always adding new features but it's not documented if it does. https://help.github.com/articles/closing-issues-via-commit-messages/

  25. laanwj merged this on Sep 8, 2015
  26. laanwj closed this on Sep 8, 2015
  27. laanwj referenced this in commit 878ea69491 on Sep 8, 2015
  28. MarcoFalke locked this on Sep 8, 2021
Contributors
caseyjonasschnellilaanwjMarcoFalkefanquakebtcdrak
Labels
GUI
Linked (view graph)
#3569 [Qt] usedSending/ReceivingAddressesAction can open multiple windows
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-21 18:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me