Skip to content
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

qt: Change uninstall icon on Windows #16760

Merged
merged 1 commit into from Aug 31, 2019

Conversation

@GChuf
Copy link
Contributor

commented Aug 29, 2019

Change uninstall icon in Windows by changing a registry value.
Original uninstall.exe icon remains the same
Reason: almost no other modern program uses that uninstall icon in Windows.

before:
before
after:
after

Copy link
Member

left a comment

utACK 635e915

@GChuf

This comment has been minimized.

Copy link
Contributor Author

commented Aug 29, 2019

As I said, the original uninstall icon remains the same. Not sure if we should change that as well - maybe with bitcoin logo all in black?

Screenshot_84

@fanquake fanquake changed the title [qt] Change uninstall icon on Windows qt: Change uninstall icon on Windows Aug 29, 2019
@fanquake fanquake added the Windows label Aug 30, 2019
@fanquake fanquake requested a review from NicolasDorier Aug 30, 2019
@GChuf GChuf force-pushed the GChuf:uninstall-icon branch from d2f51ee to 7aada05 Aug 30, 2019
@GChuf

This comment has been minimized.

Copy link
Contributor Author

commented Aug 30, 2019

I decided to add a black icon, sizes 64px - 16px, RGBA and RGB. Reason: might also come in handy in the future for something else, and the uninstall icon display will not be dependent on registry values.

about

@promag

This comment has been minimized.

Copy link
Member

commented Aug 30, 2019

Actually I think you could revert to 635e915, it just fixes the icon. Let's discuss the black icon elsewhere?

ACK 635e915.

@@ -21,7 +21,7 @@ SetCompressor /SOLID lzma
!define MUI_STARTMENUPAGE_DEFAULTFOLDER "@PACKAGE_NAME@"
!define MUI_FINISHPAGE_RUN "$WINDIR\explorer.exe"
!define MUI_FINISHPAGE_RUN_PARAMETERS $INSTDIR\@BITCOIN_GUI_NAME@@EXEEXT@
!define MUI_UNICON "${NSISDIR}\Contrib\Graphics\Icons\modern-uninstall.ico"
!define MUI_UNICON "@abs_top_srcdir@/share/pixmaps/bitcoin-black.ico"

This comment has been minimized.

Copy link
@NicolasDorier

NicolasDorier Aug 30, 2019

Member

should be \ not /

This comment has been minimized.

Copy link
@GChuf

GChuf Aug 30, 2019

Author Contributor

I don't think so. It was backslashes because that was NSIS directory which I think only appears when installing on Windows.
The pixmaps/icons are in the source folder. Take a look at how bitcoin-qt.exe icon is defined:
!define MUI_ICON "@abs_top_srcdir@/share/pixmaps/bitcoin.ico"

@GChuf GChuf force-pushed the GChuf:uninstall-icon branch from 7aada05 to 635e915 Aug 30, 2019
@GChuf

This comment has been minimized.

Copy link
Contributor Author

commented Aug 30, 2019

@promag: dropped the last commit. Will create a new PR with the icon as suggested

@l2a5b1

This comment has been minimized.

Copy link
Contributor

commented Aug 30, 2019

ACK 635e915

@practicalswift

This comment has been minimized.

Copy link
Member

commented Aug 30, 2019

ACK 635e915

Copy link
Member

left a comment

ACK 635e915 - Tested building and installing using WSL on Windows 10.

v0.18.1:
0 18 1-uninstall

This PR:
16760-uninstall

fanquake added a commit that referenced this pull request Aug 31, 2019
635e915 [qt] Change uninstall icon on Windows (GChuf)

Pull request description:

  Change uninstall icon in Windows by changing a registry value.
  Original uninstall.exe icon remains the same
  Reason: almost no other modern program uses that uninstall icon in Windows.

  before:
  ![before](https://user-images.githubusercontent.com/42591821/63967490-13a09000-ca8d-11e9-8f49-3884d33302ce.png)
  after:
  ![after](https://user-images.githubusercontent.com/42591821/63967491-13a09000-ca8d-11e9-82f0-5adb74246a1e.png)

ACKs for top commit:
  promag:
    ACK 635e915.
  l2a5b1:
    ACK 635e915
  practicalswift:
    ACK 635e915
  jonasschnelli:
    utACK 635e915
  fanquake:
    ACK 635e915 - Tested building and installing using WSL on Windows 10.

Tree-SHA512: a0f435c79e726896cc93db058f1712dcf37daefbcacbf213b340ef2d5f4ff387148b8d659302f3e84be8a9f3df23e72b0ca5937f5d77499b808081bd40bfbbac
@fanquake fanquake merged commit 635e915 into bitcoin:master Aug 31, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
sidhujag added a commit to syscoin/syscoin that referenced this pull request Aug 31, 2019
635e915 [qt] Change uninstall icon on Windows (GChuf)

Pull request description:

  Change uninstall icon in Windows by changing a registry value.
  Original uninstall.exe icon remains the same
  Reason: almost no other modern program uses that uninstall icon in Windows.

  before:
  ![before](https://user-images.githubusercontent.com/42591821/63967490-13a09000-ca8d-11e9-8f49-3884d33302ce.png)
  after:
  ![after](https://user-images.githubusercontent.com/42591821/63967491-13a09000-ca8d-11e9-82f0-5adb74246a1e.png)

ACKs for top commit:
  promag:
    ACK 635e915.
  l2a5b1:
    ACK 635e915
  practicalswift:
    ACK 635e915
  jonasschnelli:
    utACK 635e915
  fanquake:
    ACK 635e915 - Tested building and installing using WSL on Windows 10.

Tree-SHA512: a0f435c79e726896cc93db058f1712dcf37daefbcacbf213b340ef2d5f4ff387148b8d659302f3e84be8a9f3df23e72b0ca5937f5d77499b808081bd40bfbbac
@GChuf GChuf deleted the GChuf:uninstall-icon branch Sep 1, 2019
luke-jr added a commit to bitcoinknots/bitcoin that referenced this pull request Sep 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.