qt: Add workaround for QProgressDialog bug on macOS by hebasto · Pull Request #15040 · bitcoin/bitcoin

hebasto · 2018-12-26T20:27:09Z

Refs:

With this PR:

HashUnlimited · 2018-12-27T09:57:56Z

As a workaround this seems to work for the specific case, tried with extra long wallet names.

Thanks for the links btw, out of curiosity I have also tried to apply the same to a different project where Qt's dialog margins are overruled by style sheets - no effect there (this just for info).

jonasschnelli · 2018-12-30T19:21:06Z

Thanks for the fix...
But isn't it possible to add a QLayout and have the dialogue autoresize like other windows?
However, this seems to work as well.

donaloconnor

utACK 1f7ac92

hebasto · 2019-01-01T14:22:57Z

@jonasschnelli Thank you for your review.

But isn't it possible to add a QLayout and have the dialogue autoresize like other windows?

Yes, it is possible to do something like this:

QLabel* label = new QLabel(title);
progressDialog->setLabel(label);
QProgressBar* bar = new QProgressBar;
progressDialog->setBar(bar);
QPushButton* cancel_button = new QPushButton(tr("Cancel"));
progressDialog->setCancelButton(cancel_button);

QHBoxLayout* h_layout = new QHBoxLayout;
h_layout->addStretch();
h_layout->addWidget(cancel_button);
QVBoxLayout* v_layout = new QVBoxLayout;
v_layout->addWidget(label);
v_layout->addWidget(bar);
v_layout->addLayout(h_layout);
progressDialog->setLayout(v_layout);

But such implementation introduces new issues: e.g., content margins of QLayout are ignored on resize event on Linux.
Is it better to make 3-lines-of-code workaround and just wait when buggy QProgressDialog get fixed by Qt?

jonasschnelli · 2019-01-03T20:06:21Z

utACK 1f7ac92cf88ff7e6b10aaa49e174805a5823bfa2

promag · 2019-01-04T23:10:02Z

src/qt/bitcoingui.cpp

Move the implementation to something like:

void GUIUtil::PolishProgressDialog(QProgressDialog* dialog) { #ifdef Q_OS_MAC // Workaround for macOS-only Qt bug; see: QTBUG-65750, QTBUG-70357. const int margin = (dialog->fontMetrics()).width("X"); progressDialog->resize(dialog->width() + (2 * margin), progressDialog->height()); #else Q_UNUSED(dialog); #endif }

Thank you @promag. Will do.

hebasto · 2019-01-05T10:20:24Z

@promag your comment has been addressed.

Also a little of obvious code styling added.

hebasto · 2019-01-05T10:24:02Z

src/qt/bitcoingui.cpp

Note for reviewers. QProgressDialog::QProgressDialog() docs:

If QString() is passed then no cancel button is shown.

DrahtBot · 2019-01-09T22:56:43Z

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

Conflicts

No conflicts as of last run.

hebasto · 2019-01-14T17:28:22Z

Rebased.

@promag would you mind re-reviewing?

promag

utACK 2b06881, just 2 minor nits.

promag · 2019-01-14T22:47:22Z

src/qt/guiutil.cpp

nit const int margin = dialog->fontMetrics().width("X");

promag · 2019-01-14T22:47:48Z

src/qt/guiutil.cpp

nit, dialog->width() + 2 * margin.

See: QTBUG-65750, QTBUG-70357.

hebasto · 2019-01-14T23:31:01Z

@promag all nits are fixed.

hebasto · 2019-01-17T14:30:39Z

@Sjors would you mind reviewing this PR?

Sjors · 2019-01-17T15:59:45Z

tACK 7c572c4 on macOS 10.14.2

promag

utACK 7c572c4.

jonasschnelli · 2019-01-17T21:16:09Z

Tested ACK 7c572c4

7c572c4 Add workaround for QProgressDialog bug on macOS (Hennadii Stepanov) Pull request description: Fix #15016. Refs: - [QTBUG-65750: QProgressDialog too small width at larger font size on Mac](https://bugreports.qt.io/browse/QTBUG-65750) - [QTBUG-70357: QProgressDialog is too narrow to fit the text of its label](https://bugreports.qt.io/browse/QTBUG-70357) With this PR: ![screenshot from 2018-12-26 22-01-30](https://user-images.githubusercontent.com/32963518/50456571-1aa35b80-095e-11e9-8442-c285555f2bee.png) Tree-SHA512: dde668dfa7d2144973c0e868aea7fdb7d90f78584836d024ffefb8df4a709d6842fa3601954759b4462856a80e81df15b861ea39506599230a16928b621d9f8f

Sjors · 2019-01-18T18:16:36Z

@HashUnlimited that's probably too much OS-specific customisation. I don't think the benefits of that feature would outweigh the cost of maintaining it. Feel free to make a Github issue with that feature suggestion if you feel strongly about it though.

Summary: See: QTBUG-65750, QTBUG-70357. --- Backport of Core [[bitcoin/bitcoin#15040 | PR15040]] This apparently reapplies a nit fixed in D2144. I've chosen to stay close to the Core version for less merge headaches. Test Plan: unfortunately I do not have a macOS in hand to test this Reviewers: #bitcoin_abc, jasonbcox Reviewed By: #bitcoin_abc, jasonbcox Differential Revision: https://reviews.bitcoinabc.org/D6957

7c572c4 Add workaround for QProgressDialog bug on macOS (Hennadii Stepanov) Pull request description: Fix bitcoin#15016. Refs: - [QTBUG-65750: QProgressDialog too small width at larger font size on Mac](https://bugreports.qt.io/browse/QTBUG-65750) - [QTBUG-70357: QProgressDialog is too narrow to fit the text of its label](https://bugreports.qt.io/browse/QTBUG-70357) With this PR: ![screenshot from 2018-12-26 22-01-30](https://user-images.githubusercontent.com/32963518/50456571-1aa35b80-095e-11e9-8442-c285555f2bee.png) Tree-SHA512: dde668dfa7d2144973c0e868aea7fdb7d90f78584836d024ffefb8df4a709d6842fa3601954759b4462856a80e81df15b861ea39506599230a16928b621d9f8f

hebasto mentioned this pull request Dec 26, 2018

[GUI] progress window size #15016

Closed

fanquake added the GUI label Dec 26, 2018

donaloconnor approved these changes Dec 31, 2018

View reviewed changes

promag reviewed Jan 4, 2019

View reviewed changes

hebasto force-pushed the 20181226-fix-macos-qprogressdialog branch from 1f7ac92 to ce89a5c Compare January 5, 2019 10:18

hebasto commented Jan 5, 2019

View reviewed changes

DrahtBot added the Needs rebase label Jan 14, 2019

hebasto force-pushed the 20181226-fix-macos-qprogressdialog branch from ce89a5c to 2b06881 Compare January 14, 2019 17:25

DrahtBot removed the Needs rebase label Jan 14, 2019

promag reviewed Jan 14, 2019

View reviewed changes

Add workaround for QProgressDialog bug on macOS

7c572c4

See: QTBUG-65750, QTBUG-70357.

hebasto force-pushed the 20181226-fix-macos-qprogressdialog branch from 2b06881 to 7c572c4 Compare January 14, 2019 23:28

promag reviewed Jan 17, 2019

View reviewed changes

jonasschnelli merged commit 7c572c4 into bitcoin:master Jan 17, 2019

hebasto deleted the 20181226-fix-macos-qprogressdialog branch January 17, 2019 21:43

hebasto mentioned this pull request Jan 22, 2020

gui: walletcontroller showProgressDialogue functional progressBar #17978

Closed

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

Conversation

hebasto commented Dec 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HashUnlimited commented Dec 27, 2018

Uh oh!

jonasschnelli commented Dec 30, 2018

Uh oh!

donaloconnor left a comment

Choose a reason for hiding this comment

Uh oh!

hebasto commented Jan 1, 2019

Uh oh!

jonasschnelli commented Jan 3, 2019

Uh oh!

promag Jan 4, 2019

Choose a reason for hiding this comment

Uh oh!

hebasto Jan 5, 2019

Choose a reason for hiding this comment

Uh oh!

hebasto commented Jan 5, 2019

Uh oh!

hebasto Jan 5, 2019

Choose a reason for hiding this comment

Uh oh!

DrahtBot commented Jan 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

hebasto commented Jan 14, 2019

Uh oh!

promag left a comment

Choose a reason for hiding this comment

Uh oh!

promag Jan 14, 2019

Choose a reason for hiding this comment

Uh oh!

promag Jan 14, 2019

Choose a reason for hiding this comment

Uh oh!

hebasto commented Jan 14, 2019

Uh oh!

hebasto commented Jan 17, 2019

Uh oh!

Sjors commented Jan 17, 2019

Uh oh!

promag left a comment

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented Jan 17, 2019

Uh oh!

Sjors commented Jan 18, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

hebasto commented Dec 26, 2018 •

edited

Loading

DrahtBot commented Jan 9, 2019 •

edited

Loading