qt: Remove menu icons #16612

pull laanwj wants to merge 1 commits into bitcoin:master from laanwj:2019_08_remove_menuicons changing 16 files +19 −93
  1. laanwj commented at 2:14 PM on August 14, 2019: member

    Remove the icons from the application menu. Why remove?

    • They are inconsistently applied, some actions had icons, some newer ones don't. Good luck coming up with a sensible icon for everything
    • Menu icons don't seem to have a place in modern UI: for example, GNOME, MacOS have stopped showing these a long time ago (see #16584 (comment))
    • Less bikeshedding opportunity about "what should the icon for this be"

    Removed icons:

    /icons/quit            res/icons/quit.png
/icons/about           res/icons/about.png
/icons/about_qt        res/icons/about_qt.png
/icons/options         res/icons/configure.png
/icons/key             res/icons/key.png
/icons/verify          res/icons/verify.png (also .svg)
/icons/debugwindow     res/icons/debugwindow.png
/icons/open            res/icons/open.png
/icons/info            res/icons/info.png
/icons/filesave        res/icons/filesave.png

    I checked that these icons are used nowhere else.

    Removed from the menu not removed from the repository, because still referenced by other parts of the code:

    /icons/lock_closed
/icons/edit
/icons/address-book
/icons/send
  2. laanwj added the label GUI on Aug 14, 2019
  3. practicalswift commented at 2:41 PM on August 14, 2019: contributor

    Concept ACK

  4. hebasto commented at 4:37 PM on August 14, 2019: member

    Concept ACK

  5. DrahtBot commented at 4:37 PM on August 14, 2019: member

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #15529 (Add Qt programs to msvc build (updated, no code changes) by sipsorcery)
    • #15450 ([GUI] Create wallet menu option by achow101)
    • #15202 (gui: Add Close All Wallets action by promag)
    • #9849 (Qt: Network Watch tool by luke-jr)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  6. MarcoFalke added the label Needs gitian build on Aug 14, 2019
  7. MarcoFalke commented at 5:50 PM on August 14, 2019: member

    Before (x11 Gnome / wayland): Screenshot from 2019-08-14 13-45-14 Screenshot from 2019-08-14 13-45-32

    After (x11 Gnome / wayland): Screenshot from 2019-08-14 13-43-35 Screenshot from 2019-08-14 13-44-12

  8. MarcoFalke commented at 5:58 PM on August 14, 2019: member

    ACK e181eb2b19c5bdccd36e7fd84d928b561ef98011

    <details><summary>Show signature and timestamp</summary>

    Signature:

    -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK e181eb2b19c5bdccd36e7fd84d928b561ef98011
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUjijQv/WDprJlrAN4OhF4JNEnYn9CJqzuzCYgv1Vwvl/IkEa70NiJzcXO0r9pS2
+rWz/ysMadkiSnqYyTGBAAX9NXU7R/wiY0z2YtwgE0+gyo0yFqd4EwQB7g1pvZfB
G103EG8mOiDrZ6ZgZn9aFlRwAevUx2QXkad4V9JT4ssXgI6SYwsuQh4H7/c+iOj0
zQf87oyi1SVTJgFOUAP72q45D83fm09tGwhZTGnaMaAmFr9pMVpCwVeR0hMxVveD
CA3+VNS2CbPq5l4++pjK7FO6lWM9tW74Gkv1slv9iGb4sPdKihsOv35/OSEZOtwP
SO5vp1RfieLCXSVUaohm0uP4bS3QnT8VL8SDTdXscE1md9NU9ycyVXjxlsYC8SiF
qoxyL1fHI4SJVrJr5t+tP2YMZCvbl7AFrd2Qg6dX7J8E07/ZzQLY/AJ0Ul1JPR4A
eaIp5S+juXvBLY9ZReYwJCIHf0BF8B3mNKphot48CE9/wJsvdlqsubiQOC7Eho04
WU6bV19x
=ld0Q
-----END PGP SIGNATURE-----

    Timestamp of file with hash 1104ade14aa4e0052c1230cc7604a8017dc830605d57010203b11cabe43e401a -

    </details>

  9. emilengler commented at 6:02 PM on August 14, 2019: contributor

    Concept NACK what is with people who can't read? When I was in kindergarten and primary school these icons were essential for me (especially in english software). Also in countries like Venezuela where Bitcoin is used a lot, 5% of the population cannot read and Bitcoin should be accessible for anyone no matter which education he got

  10. hebasto commented at 6:22 PM on August 14, 2019: member

    ACK e181eb2b19c5bdccd36e7fd84d928b561ef98011, tested on Linux Mint 19.2 + Cinnamon 4.2.3:

    before: Screenshot from 2019-08-14 21-10-59-0

    with e181eb2b19c5bdccd36e7fd84d928b561ef98011 (dark theme): Screenshot from 2019-08-14 21-20-16-0

    with e181eb2b19c5bdccd36e7fd84d928b561ef98011 (light theme): Screenshot from 2019-08-14 21-39-37-0

  11. MarcoFalke commented at 6:26 PM on August 14, 2019: member

    @hebasto It changes the background color as well?

  12. hebasto commented at 6:35 PM on August 14, 2019: member

    It changes the background color as well?

    No, it does not. The former build is gitian and the latter one is compiled against system Qt. And my DE has dark theme activated. My comment has been updated.

  13. promag commented at 6:38 PM on August 14, 2019: member

    Concept ACK!

  14. GChuf commented at 6:42 PM on August 14, 2019: contributor

    Agree with @emilengler about people who have problems with English language or don't have the option to use bitcoin-qt in their own language. On the other hand I agree with @laanwj about inconsistency and so called bikeshedding.

    I think it's worth keeping them, and if we do, I'd be glad to help with creating/searching for the missing icons.

  15. laanwj commented at 6:44 PM on August 14, 2019: member

    Concept NACK what is with people who can't read?

    QtAccessibility and text-to-speech would probably be the best in that case, as it also helps people who are blind. I think this is far outside the scope of this issue though.

    (No, the little icons besides menu items don't actually help people that can't read in any significant way, do you think they could navigate the Send interface, or the transaction list right now? What about the tsend confirmation dialog?)

  16. promag commented at 6:58 PM on August 14, 2019: member

    Concept ACK!

  17. fanquake commented at 12:23 AM on August 15, 2019: member

    Concept ACK. Agree with all of @laanwj's points. I think this repository has far higher priority things to do than be worrying about / arguing over dropdown menu icons.

    As I mentioned in the other PR. I don't think I've seen these icons on macOS for quite some time, if ever?

    master (f418c3379cbad8caaf3e988caf727fe62b218f8c): master

    this PR (e181eb2b19c5bdccd36e7fd84d928b561ef98011): no-icons

  18. DrahtBot added the label Needs rebase on Aug 15, 2019
  19. jonasschnelli commented at 6:04 AM on August 15, 2019: contributor

    ACK e181eb2b19c5bdccd36e7fd84d928b561ef98011 (needs rebase though)

  20. GChuf commented at 10:31 AM on August 15, 2019: contributor

    NACK. Recognizing icons is much faster than reading the menus, there's studies about that. I don't read the menu, I look at the icons - maybe some of you do too. The send interface I can also imagine navigating without being able to read (english) with the help of icons that are there, though of course I get laanwj's point. We aren't here to solve the problem of illiteracy. But if your point is "there's too much problems with icons so we'll remove them", then I very much regret making that filesave PR which (in?)directly led to this. I think people's opinion differs from the developers' perspective and a lot of people will miss the icons if they're gone.

  21. laanwj commented at 10:52 AM on August 15, 2019: member

    The send interface I can also imagine navigating without being able to read (english)

    I think equating not being able to read English with illiteracy (and dyslexia etc) is wrong. There's a whole, active project at transifex (https://www.transifex.com/bitcoin/bitcoin/) to provide translations and localizations of the GUI in tons of different languages. If there isn't a reasonable translation in your language, please help update it.

    I'm all for accessibility for disadvantaged people, and if you want to work on that, that'd be wonderful. But I've never seen PRs from you to improve accessibility otherwise, so please, don't use them as a pretense to make this specific point about style.

  22. laanwj force-pushed on Aug 15, 2019
  23. qt: Remove menu icons 390874c722
  24. laanwj force-pushed on Aug 15, 2019
  25. promag commented at 11:29 AM on August 15, 2019: member

    Could someone try QApplication::instance()->setAttribute(Qt::AA_DontShowIconsInMenus)? If it works we could avoid deleting icons for now.

  26. GChuf commented at 11:29 AM on August 15, 2019: contributor

    Just to make sure: I mean and I meant no disrespect whatsoever to anyone here.

    I didn't want to equate illiteracy with anything else. Perhaps I phrased it wrong. I am active at transifex and I am helping with translating into my native language. I've also made an issue #16591 regarding some translations.

    Regarding my contributions, I've only just started contributing a few days ago. My point is, I think this PR would hurt accessibility in some ways, and is not about style. We obviously disagree.

  27. laanwj commented at 11:33 AM on August 15, 2019: member

    Could someone try QApplication::instance()->setAttribute(Qt::AA_DontShowIconsInMenus)? If it works we could avoid deleting icons for now.

    I think deleting the icons would be a feature, not a problem, when they're unused. They'd still be in git history.

    I think this is too controversial, closing. remind me not to get involved in UI issues.

  28. laanwj closed this on Aug 15, 2019
  29. Sjors commented at 11:33 AM on August 15, 2019: member

    There might be a way to avoid bike-shedding over icons and yet keep them, which is to use QIcon fromTheme. That would still involve removing the existing icon set.

    Apple's Human Interface Guidelines recommend mostly avoiding menu icons, or at least using highly standardized ones: <img width="749" alt="Schermafbeelding 2019-08-15 om 13 20 18" src="https://user-images.githubusercontent.com/10217/63091676-70fdf280-bf5f-11e9-9c8d-4e3e9638357a.png">

    They generally know what they're doing, although unfortunately they don't really explain the motive here. Interestingly Apple uses icons extensively inside application windows, just not in the menu. I guess the moral is that important stuff shouldn't be exclusively in the menu.

    Since we do still use icons in the application windows, removing them from the menu doesn't fully stop bike shedding.

  30. DrahtBot commented at 12:18 PM on August 15, 2019: member

    <!--a722867cd34abeea1fadc8d60700f111-->

    Gitian builds for commit a7aa809027633556dd3280c6e29ca98eb3235a3d (master):

    Gitian builds for commit 37334c7ad139cc6c8a683d8b63e619e5770009ea (master and this pull):

  31. DrahtBot removed the label Needs gitian build on Aug 15, 2019
  32. MarcoFalke commented at 2:39 PM on August 15, 2019: member

    ACK on either removing them or replacing them with familiar icons, as per #16612 (comment)

    Working against common design guidelines doesn't make Bitcoin Core more accessible

  33. practicalswift commented at 3:11 PM on August 15, 2019: contributor

    ACK 390874c722c5af0b37cb94151ffb49986eab5f7d -- diff looks correct

    Working against common design guidelines doesn't make Bitcoin Core more accessible

    Agree fully.

  34. l2a5b1 commented at 4:42 PM on August 15, 2019: contributor

    Please reopen this pull request.

    ACK 390874c722c5af0b37cb94151ffb49986eab5f7d - Bitcoin Core has a very simple application menu. As long as the menu items describe their actions clearly and unambiguously then the icons alongside the label are redundant and offer very little value, if anything at all.

    A good application menu just needs menu labels that unambiguously and clearly describe each action.

    Using icons alongside a menu label to describe an action may make sense when

    • it is not possible to unambiguously and succinctly describe menu actions. The icon alongside the menu label can then provide hints to the user.

    • an application has a (configurable) toolbar and application menu so that the items on each one are analogous to each other (usually the case when there are a lot of actions that the user can perform).

    • When menu actions in a menu section represent a collection to a single resource type, for example bookmarks to web pages in a web browser.

    UX decisions:

    It would be great if we could validate UX decisions, including this one, via usability tests not subjective ACKS or NACKS. @moneyball Is Square Crypto already conducting usability tests of the Bitcoin Core software and Bitcoin UX in general?

    As far as bikeshedding is concerned:

    Every person here contributes to Bitcoin Core for its own reasons. Some care deeply about strong conformance to best C++ practices; others about code style; or UX design; or cryptography and what have you. I think pull requests and constructive or passionate input regarding whatever Bitcoin Core development related topic should be welcomed.

  35. MarcoFalke reopened this on Aug 15, 2019
  36. moneyball commented at 4:54 PM on August 15, 2019: contributor

    @l2a5b1 Square Crypto is still in the process of hiring a designer. I expect this to take awhile. We will then need to determine prioritization of projects, and I am not sure if the Core wallet will be at the top of the list. However, one of our goals is to help catalyze and grow a design community for Bitcoin, so that there is more design talent available and interested in helping many projects including the Core wallet.

  37. DrahtBot removed the label Needs rebase on Aug 15, 2019
  38. GChuf commented at 5:47 PM on August 15, 2019: contributor

    +1 on usability testing. Users' perspective should be important.

    It might be that programmers and developers see software in another light than end users. For me, icons are a faster way of recognizing things - as mentioned before, humans recognize pictures much faster than words. I also don't know what the "common design guidelines" would be. Are any people here actual designers?

    An option would be to keep the icons and give the user a chance to toggle them back on if they want. But I think we should ask users what they think.

  39. Sjors commented at 8:27 PM on August 15, 2019: member

    toggle them back on

    That's a net increase in code complexity; we'd still need to bike shed over icons, but now also maintain and test an additional setting.

    Good UI is very hard to do by group consensus. This is why #10102 (multiprocess bitcoin) is an important development. It would ultimately allow alternative GUI implementations to be first class citizens of the Bitcoin Core wallet. Right now any alternative GUI would have to use the RPC, which is not powerful enough (although projects like BTCPay do use it).

    It's important to conserve review time & energy for important-but-hard stuff like multi-process bitcoin. It's one thing to have many small pull requests, but it's a whole other thing for those pull requests to draw lots of energy from bike-shedding, which is inevitable when touching the UI (been there, see #11605).

  40. promag commented at 11:09 PM on August 15, 2019: member

    Could someone try QApplication::instance()->setAttribute(Qt::AA_DontShowIconsInMenus)? If it works we could avoid deleting icons for now.

    I think deleting the icons would be a feature, not a problem, when they're unused. They'd still be in git history. @laanwj that's not my point. QAction can be associated to multiple widgets (1) and displayed differently - I don't know if we have these. My point is that we could first hide menu icons only and keep the actions with icons in the case those are associated to other widgets. Note that this is a Qt feature.

    That said I don't think icons in menu items improve accessibility - well structured menus, unambiguous labels and shortcuts do improve accessibility.

    My personal opinion is that menus without icons look more clean and professional.

    1 - see https://doc.qt.io/qt-5/qwidget.html#addAction and https://doc.qt.io/qt-5/qaction.html#associatedWidgets

  41. kallewoof commented at 4:12 AM on August 16, 2019: member

    ACK 390874c722c5af0b37cb94151ffb49986eab5f7d

    I don't buy that it's faster to look at icons vs reading text, and as mentioned, accessibility support is the way to go for people with special needs. Translations is the way to go for people who don't speak English. (Come on, having people use a tool in a language they don't understand to transfer money around? Let's not encourage that at all.)

  42. jonasschnelli commented at 6:22 AM on August 16, 2019: contributor

    utACK 390874c722c5af0b37cb94151ffb49986eab5f7d

  43. jonasschnelli merged this on Aug 16, 2019
  44. jonasschnelli closed this on Aug 16, 2019
  45. jonasschnelli referenced this in commit 93bacb8cc9 on Aug 16, 2019
  46. fanquake referenced this in commit 8bd46ad9c8 on Aug 22, 2019
  47. fanquake referenced this in commit 90ee6bb651 on Aug 22, 2019
  48. fanquake referenced this in commit bca388db0d on Aug 22, 2019
  49. luke-jr commented at 8:59 AM on August 22, 2019: member

    Post-humourous concept NACK: Apple doesn't dictate design guidelines for anything other than macOS. On other systems, removing the icons contradicts application expectations.

    (But if nobody is willing to do the work to fix them, maybe it's for the best...)

  50. GChuf commented at 2:07 PM on August 22, 2019: contributor

    I'm still willing to help with the icons.

    To address the initial reasons for removal, I'm sure there are sensible icons out there for every menu option and inconsistency could be solved. Of course the new proposed icons might spark up a debate again, which leads us to bikeshedding - however, the problem might not lie in the icons themselves, but in people willing to argue over something below their skill level and of low priority, even though they consider the icons useless (we did remove them).

    Whether or not you believe images to be faster or slower than text ("believe" being the operative word), I think it would be sensible to agree that images are (or can be) complementary to text. And regarding accessibility: although icons don't solve every problem, they could help people with dyslexia, poor vision, learning english, and so on.

  51. luke-jr commented at 1:40 AM on August 23, 2019: member

    Well, if you're willing to do the work, please open a PR adding the new icons back in. :)

    Note that one unfortunately-common "gotcha" is that icons should be licensed under MIT or MIT-like terms.

  52. jonasschnelli referenced this in commit d72758c3f6 on Aug 23, 2019
  53. sidhujag referenced this in commit 4488a1b27e on Aug 23, 2019
  54. HashUnlimited referenced this in commit c7c8acd83c on Aug 30, 2019
  55. fanquake referenced this in commit b0e268d2f6 on Oct 1, 2019
  56. sidhujag referenced this in commit 32d3f3bf35 on Oct 2, 2019
  57. Fabcien referenced this in commit c7066157da on Dec 24, 2020
  58. PastaPastaPasta referenced this in commit ad5dd2a8b3 on Sep 21, 2021
  59. kittywhiskers referenced this in commit d47998a5f2 on Oct 12, 2021
  60. MarcoFalke locked this on Dec 16, 2021
Contributors
laanwjpracticalswifthebastoDrahtBotMarcoFalkeemilenglerpromagGChuffanquakejonasschnelliSjorsl2a5b1moneyballkallewoofluke-jr
Labels
GUI
Linked (view graph)
#16584 gui: New filesave icon, updated copyright#16677 gui: remove unused PlatformStyle::TextColorIcon#16969 refactor: Remove Qt function to disable menu icons on macOS
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-13 18:14 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me