As menu icons were removed in #16612, this removes an unnecessary function for macOS Could this get into v0.19.0?
refactor: Remove Qt function to disable menu icons on macOS #16969
pull emilengler wants to merge 1 commits into bitcoin:master from emilengler:2019-09-refactor-qt-menuicons-macos changing 1 files +0 −3-
emilengler commented at 5:43 PM on September 26, 2019: contributor
-
refactor: Remove Qt function to disable menu icons on macOS 3eea6a8f26
- laanwj added the label GUI on Sep 26, 2019
-
MarcoFalke commented at 6:34 PM on September 26, 2019: member
What is the risk of leaving it in?
-
emilengler commented at 6:37 PM on September 26, 2019: contributor
@MarcoFalke There is no risk but what's the point of leaving useless code in there? It just makes the program slower and bigger (even if there is no notable change)
- MarcoFalke added the label Refactoring on Sep 26, 2019
- fanquake added the label macOS on Sep 26, 2019
-
promag commented at 10:10 PM on September 26, 2019: member
I believe this also affects context menus, not sure if we are using them with icons?
Edit: apparently not, for instance:
And
-
laanwj commented at 1:51 PM on September 27, 2019: member
I didn't touch context menus in my PR at least.
-
emilengler commented at 2:04 PM on September 27, 2019: contributor
-
jonasschnelli commented at 1:03 AM on September 30, 2019: contributor
utACK 3eea6a8f2686352a0fd868fee6af42ef1283bdda AFAIK context menu icons are not shown on macOS, also not when starting with
-uiplatform=other(orwindows). -
laanwj commented at 10:00 AM on September 30, 2019: member
On my X11 GTK system I don't have any icons in context menus though.
Do we even have icons in context menus? I don't think so. Mind that "getImagesOnButtons()" is about images on buttons, not menus.
-
promag commented at 10:07 AM on September 30, 2019: member
Mind that "getImagesOnButtons()" is about images on buttons, not menus.
Yup, I got it wrong while reading https://github.com/bitcoin/bitcoin/blob/79aeed8e76e8bc70722e6178678b0b3840694d5c/src/qt/addressbookpage.cpp#L67-L70
I thought these actions were for the context menu, but actually they are buttons.
-
promag commented at 2:04 PM on September 30, 2019: member
ACK 3eea6a8f2686352a0fd868fee6af42ef1283bdda.
- fanquake approved
-
fanquake commented at 2:09 AM on October 1, 2019: member
ACK 3eea6a8f2686352a0fd868fee6af42ef1283bdda
- fanquake referenced this in commit b0e268d2f6 on Oct 1, 2019
- fanquake merged this on Oct 1, 2019
- fanquake closed this on Oct 1, 2019
- emilengler deleted the branch on Oct 1, 2019
- sidhujag referenced this in commit 32d3f3bf35 on Oct 2, 2019
- Fabcien referenced this in commit 75d4f4c0c8 on Dec 24, 2020
- PastaPastaPasta referenced this in commit ad5dd2a8b3 on Sep 21, 2021
- kittywhiskers referenced this in commit d47998a5f2 on Oct 12, 2021
- DrahtBot locked this on Dec 16, 2021
Labels