Added Window to menu bar with Minimize action #5424

pull blakejakopovic wants to merge 1 commits into bitcoin:master from blakejakopovic:window-menu changing 2 files +16 −0
  1. blakejakopovic commented at 1:02 PM on December 4, 2014: contributor

    By convention, all native OS X apps have a Window menu with Minimize, Zoom, and Bring All to Front actions. Additionally, applications make use of the ⌘+M hotkeys for minimizing. This PR is primarily targeted toward enhancing usability for every day Mac users.

    To keep the code simple, and for greater interface consistency, I have submitted this as a platform independent feature.

    This PR brings in a few additional translations. I'm happy to submit an update with the required bitcoin_en.ts updates. make translate seems currently broken in master.

  2. Added Window to menu bar with minimize b4cc08dc53
  3. laanwj commented at 1:18 PM on December 4, 2014: member

    No need to update the translation files - we'll do that periodically, it would possibly complicate merging otherwise. Though I wonder why "make translate" is broken for you?

  4. fanquake commented at 1:46 PM on December 4, 2014: member

    Built with Qt 5.3.2 on OS X 10.9.5. Menu appears correctly. ⌘+M works as expected. screen shot

  5. blakejakopovic commented at 1:49 PM on December 4, 2014: contributor

    @laanwj I hadn't looked into it, but it was file permission related. I've since pulled the latest master, run make clean, and it seems to be working again. I'll keep an eye out for it breaking again.

  6. laanwj added the label Mac on Dec 4, 2014
  7. Diapolo commented at 4:54 PM on December 4, 2014: none

    If this is Mac only shouldn't the changes be guarded by an #ifdef?

  8. sipa commented at 7:32 PM on December 4, 2014: member

    @Diapolo doesn't look like the changes are intended to be Mac-only.

    Do you think this is not an improvement for other platforms?

  9. jonasschnelli commented at 8:20 PM on December 4, 2014: contributor

    Just tested on OSX 10.10 and Ubuntu Linux 14.04. Generally it works. But IMO it's not worth having a "window" menu with one menu item (minimize). Users can use the minimize-window-icon (i guess this is available on windows too) or even use the Dock/Tray-Context-Menu.

    I see no need for this feature.

  10. blakejakopovic commented at 11:03 PM on December 4, 2014: contributor

    @Diapolo I felt that it's general enough to make sense for all platforms. They all support minimize, but as a primarily OS X user, that's open for discussion.

    The Windows KB does have a standard shortcuts list.

    Microsoft Natural Keyboard keys

    • Windows Logo+M: Minimize all

    I believe the Qt::CTRL key on Windows represents the Windows Logo. But might be worth double checking. @jonasschnelli Presently the key advantage is the hotkey. OS X users tend to become extremely efficient using shortcuts, and the Window menu gives that feature a home. The Show/Hide features really don't make sense for Mac users and break workflow.

    I see this PR as being the foundation of other Window related actions. If you're a Mac user, I bet you have at least 10 apps open, so take a quick look what they have in their menu. Email clients and browsers are good examples. You'll likely find some useful navigation related actions, with hotkeys. I see this as being a usability, and accessibility win.

    In terms of what to include for each platform, if the Window menu is approved, I guess it will require further discussion. Personally I see their being many actions that would be useful for Mac users, such as ⌘+1, ⌘+2, ⌘+3, ⌘+4 to jump between wallet tabs (just like browsers). However, some of these actions might be best suited to be platform specific.

  11. fanquake commented at 2:56 AM on December 5, 2014: member

    While being able to use Command+M to minimize the window is handy, I somewhat agree with @jonasschnelli about adding the window menu item just for minimize.

    Why not add the 'Bring All to Front' and 'Zoom' items as part of this pull? I was also going to make the accessibility point, but it looks like you bought it up.

  12. blakejakopovic commented at 3:14 AM on December 5, 2014: contributor

    @fanquake 'Bring All to Front' and 'Zoom' would be OS X specific, and as a first step (and my first code commit), I opted to keep it simple. I'm not opposed to adding the above actions, nor others that make sense - however, I feel that they require a bit more thought and discussion. Cross-platform actions may be limited to what Qt supports.

    As I stated above, I see this as a foundational commit to mark the way forward for better accessibility. I could add all actions under the sun, but that ends up just bloating the project. If there is reasonable need, and general consensus, I see future commits adding both cross-platform and platform targeted actions to the menu.

  13. jonasschnelli commented at 6:41 AM on December 5, 2014: contributor

    Would it be possible to support the cmd-key shortcut without the menu? Have a look at the debugwindow code. There is also a key press handling implemented (ESC).

    IMO: OSX Users tend to use "mission control", "hot corners" and other window zooming/shuffling concepts rather than minimizing every single window.

    ACK on a shortcut-only-solution NACK on a single item menu

  14. jonasschnelli commented at 6:42 AM on December 5, 2014: contributor

    Should not be tagged as Mac. Needs GUI tag instead.

  15. laanwj commented at 7:16 AM on December 5, 2014: member

    Other platforms have their own shortcut keys, as well as window decorations for minimization. Generally these are built into the window system, so don't have to be fiddled with by applications. Rather not, even, because the user may decide to change them at the system level.

    And the point of Qt is to not have to micromanage these things.

    So I tend towards NACK. If this is really truly useful for Mac (I leave that to Mac users) it could be done as a mac-only feature.

  16. blakejakopovic commented at 12:33 PM on December 5, 2014: contributor

    Shortcuts only exist if people have a place that they can learn them - otherwise they remain unknown. If you look at almost any application, Mac/Linux/Windows, they will have an Edit menu with cut CTRL + X, copy CTRL + C and paste CTRL + V (action + shortcut). There is also a long standing precedent that users can browse application menus when they are stuck, to find the right commands and actions. It makes little sense for shortcuts to exist without menu items.

    I acknowledge that a menu with only one item adds limited value - but this is an initial commit aimed to get the ball rolling. The worst type of programming is pretending to know what's needed before you have first tested the waters. This is a case of easing in, and seeing what new value can be added. I don’t think anyone has complained about the addition of the minimize hotkey; which to me suggests that it is indeed worth having.

    The risk is that we add a main menubar item that remains relatively unused - which is an extremely low risk. If it truly sucks, or doesn’t make sense (for any or all platforms) in three months, just remove it. At the end of the day, this adds a commonly used action, to a menu that people who are looking for something will browse.

    Keep in mind that in reality, most people don't know what 'hot corners' are, nor a lot of other fancy window management tools. They understand windows, menus and minimizing applications.

  17. blakejakopovic commented at 12:39 PM on December 5, 2014: contributor

    For reference, here is a skeleton Xcode OS X app menu. Apple, largely praised for their designs and usability, by default have 6 (+1) main menu bar items for native applications. screen shot 2014-12-05 at 8 24 42 pm

  18. jonasschnelli commented at 12:42 PM on December 5, 2014: contributor

    Maybe a #ifdef mac (OSX only) version with...

    • Minimize
    • Zoom
    • ////LINE
    • Bring All to Front

    ... would make sense?

    Bring All to Front should also bring the RPC/Debug window to front.

  19. luke-jr commented at 12:54 PM on December 5, 2014: member

    If this is an OS convention, it should really be handled by the OS (or at least by Qt), not every application. That being said, it's reasonable for us to have a workaround when it isn't - but it shouldn't be pushing a Mac convention on non-Mac platforms in the process. KDE and Windows, at least, have their conventional "window menu" in the application icon, and I imagine that works just fine already (I can confirm it does for KDE).

  20. laanwj commented at 1:07 PM on December 5, 2014: member

    Pushing a MacOSX convention on other platforms just doesn't make sense, no matter how "praised" Apple is. That's the last from me on this subject for now. I agree with @luke-jr for a change.

  21. Diapolo commented at 1:19 PM on December 5, 2014: none

    The whole discussion here is why I asked if that should be Mac specific, it's UX-wise nonsense on Windows IMHO.

  22. blakejakopovic commented at 1:29 PM on December 5, 2014: contributor

    I felt like this was a positive step forward, but seemingly not. Feel free to close the PR.

  23. luke-jr commented at 1:35 PM on December 5, 2014: member

    @blakejakopovic Prefer if you can fix it to address comments...

  24. laanwj commented at 1:40 PM on December 5, 2014: member

    Improving the MacOSX interface for MacOSX users is a perfectly welcome thing. No one is negative about that.

    BTW

    Personally I see their being many actions that would be useful for Mac users, such as ⌘+1, ⌘+2, ⌘+3, ⌘+4 to jump between wallet tabs (just like browsers).

    I don't have a ⌘ key, but at least here on Ubuntu that already works w/ Alt-1..4. If it doesn't work for Mac, it should indeed be fixed for Mac.

  25. blakejakopovic commented at 2:00 PM on December 5, 2014: contributor

    Option/Alt + 1/2/3/4 works on Mac, except for going from opt-3 to opt-2, as the label field automatically focuses, and opt-2 outputs the char. ⌘ was only ever an example of what Qt::CTRL represents while using a specific OS. But, to my point, I had no clue these existed - and using Option/Alt feels clunky on OS X.

    I don't see the cost/benefit being positive by using a bunch of #ifdef for Mac. Each new line of code just adds maintenance, and greater complexity for minimal gain. Perhaps wallet enhancements are best left until after the wallet code is separated out. If I have an idea on how to rethink the implementation, I'll submit a new PR at that point in time.

  26. laanwj commented at 2:04 PM on December 5, 2014: member

    It doesn't have to be #ifdef everywhere. You could for example have one #ifdef in the main file, then pass a GuiPlatform enumeration down into window constructors that determines the mode. This would make sure all the code is compiled for all platforms, not ridding the source with #ifdefs, but MacOSX-specific tweaks appear on MacOSX only. You could then even test the functionality on other platforms with a hidden flag. (we've started doing the same for disablewallet mode in some places)

  27. laanwj added the label GUI on Dec 10, 2014
  28. jonasschnelli commented at 3:11 PM on January 9, 2015: contributor

    @blakejakopovic Needs rebase.

    This pull currently tend to NACK. I guess a shortcut only solution according to @laanwj way of handling the platform dependent code could get more ACKs.

  29. blakejakopovic commented at 1:41 AM on January 12, 2015: contributor

    I looked into making this a platform specific PR, as well as QT features for Mac. QT doesn’t seem to natively support the Zoom feature, nor Bring All to Front. For reference, the QT Creator app source just toggles zoom between maximised and normal, and doesn’t support Bring All to Front. Feel free to correct me if I missed something.

    The Zoom action is meant to toggle between user selected window size, and one that optimally suits the window content (without scrollbars, if possible). It seems fairly redundant for the current app design.

    The Bring all to front can be implemented, but it will be fragile to the addition of new windows with a custom implementation. It would also require making decisions around sub-widgets layering, such as the debug window, command-line-options, edit label, etc.

    So this ends up leaving the Window menu effectively empty (with only the Minimise action). However, there are other possible actions that make sense, such as actions to jump between tabs.

    While I think that there is decent room for improvement in the usability and accessibility features of Bitcoin Core, specifically for Mac, I feel that perhaps these efforts would have a greater impact on more user targeted wallet applications. I don’t wish to take this PR any further - but others are welcome.

  30. blakejakopovic closed this on Jan 12, 2015

  31. MarcoFalke locked this on Sep 8, 2021

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-15 15:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me