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

Remove unused "What's This" button in dialogs on Windows OS #85

Merged
merged 2 commits into from
Jan 28, 2021

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Sep 7, 2020

Fix #74.

From Qt docs:

The widget flags f are passed on to the QWidget constructor. If, for example, you don't want a What's This button in the title bar of the dialog, pass Qt::WindowTitleHint | Qt::WindowSystemMenuHint in f.

Screenshot on Windows 10 (2004):

@maflcko
Copy link
Contributor

maflcko commented Sep 7, 2020

Please split this into two commits, otherwise literally every commit could be a scripted diff by including the diff as a patch in the commit body

-BEGIN VERIFY SCRIPT-
git grep -l 'QDialog(parent)' -- src/qt | xargs sed -i -E 's/QDialog\(parent\)/QDialog\(parent, GUIUtil::dialog_flags\)/g'
-END VERIFY SCRIPT-
@hebasto
Copy link
Member Author

hebasto commented Sep 7, 2020

Updated e322fe7 -> ac7ccd6 (pr85.01 -> pr85.02, diff):

Please split this into two commits, otherwise literally every commit could be a scripted diff by including the diff as a patch in the commit body

@Bosch-0
Copy link

Bosch-0 commented Oct 23, 2020

tACK ac7ccd6 Tested on Windows 10.0.18363 Build 18363.

Master (56a461f) vs this PR (ac7ccd6)

Group 109

This PR also removes this unused dialog button from the following windows on Windows 10 compared to master.

  • About Bitcoin Core
  • Command-line options
  • Request payment modal when generating a new receive address
  • Create Wallet
  • Encrypt wallet when creating a new wallet
  • Signature signing / verifying
  • Open Bitcoin URI
  • Receive addresses
  • Sending addresses

@luke-jr
Copy link
Member

luke-jr commented Oct 24, 2020

Is there a way we can exclude the "What's This?" button rather than explicitly list included buttons?

@hebasto
Copy link
Member Author

hebasto commented Oct 24, 2020

Is there a way we can exclude the "What's This?" button rather than explicitly list included buttons?

Qt docs do not provide that way.

@jonasschnelli jonasschnelli added this to the 0.22.0 milestone Nov 11, 2020
Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review ACK ac7ccd6 but with some suggestions.

Does it make sense to have a QDialog subclass? There's also the call handleCloseWindowShortcut.

@@ -53,7 +53,7 @@ int getIndexForConfTarget(int target) {
}

SendCoinsDialog::SendCoinsDialog(const PlatformStyle *_platformStyle, QWidget *parent) :
QDialog(parent),
QDialog(parent, GUIUtil::dialog_flags),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It extends dialog but it's not used as such. I think you could drop this change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could drop this change?

Adding widget flags to remove "What's This" button is the goal of this PR. Or did I understand you in a wrong way?

@@ -21,7 +22,7 @@
#include <QTextDocument>

ReceiveCoinsDialog::ReceiveCoinsDialog(const PlatformStyle *_platformStyle, QWidget *parent) :
QDialog(parent),
QDialog(parent, GUIUtil::dialog_flags),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as SendCoinsDialog.

Copy link
Contributor

@jonasschnelli jonasschnelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK ac7ccd6

@jonasschnelli jonasschnelli merged commit 68692d3 into bitcoin-core:master Jan 28, 2021
@hebasto hebasto deleted the 200907-whatsit branch January 28, 2021 09:50
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 28, 2021
…ogs on Windows OS

ac7ccd6 scripted-diff: Remove unused "What's This" button in dialogs on Windows (Hennadii Stepanov)
b695148 qt: Add flags to prevent a "What's This" button on Windows OS (Hennadii Stepanov)

Pull request description:

  Fix #74.

  From [Qt docs](https://doc.qt.io/qt-5/qdialog.html#QDialog):
  > The widget flags _f_ are passed on to the `QWidget` constructor. If, for example, you don't want a **What's This** button in the title bar of the dialog, pass `Qt::WindowTitleHint | Qt::WindowSystemMenuHint` in _f_.

  Screenshot on Windows 10 (2004):
  - master (3ba25e3)
  ![Screenshot from 2020-09-07 16-55-42](https://user-images.githubusercontent.com/32963518/92402384-20dc6a00-f138-11ea-9dcb-3e0f6373ff22.png)

  - this PR (e322fe7e19ac504272d14b9b4f9b28b13df888ed)
  ![Screenshot from 2020-09-07 18-31-16](https://user-images.githubusercontent.com/32963518/92402509-5aad7080-f138-11ea-8b63-9bbbf8b9b9e1.png)

ACKs for top commit:
  Bosch-0:
    tACK ac7ccd6 Tested on Windows 10.0.18363 Build 18363.
  promag:
    Code review ACK ac7ccd6 but with some suggestions.
  jonasschnelli:
    utACK ac7ccd6

Tree-SHA512: f6750a17b7203106cb4db5870becba1cef6a505d4edcc710ba131338bd3aae051510627e62c9bcb8345a7f497c614709e11aeb8f6ae3ea85967bbce2a8c69e64
@hebasto
Copy link
Member Author

hebasto commented Feb 24, 2021

@luke-jr

Is there a way we can exclude the "What's This?" button rather than explicitly list included buttons?

I was wrong.

The Qt::AA_DisableWindowContextHelpButton application attribute value was added in Qt 5.10.

Unfortunately, we have minimum Qt version 5.5.1.

@bitcoin-core bitcoin-core locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Help '?' button on create wallet and encrypt wallet modals does nothing
6 participants