-
Notifications
You must be signed in to change notification settings - Fork 288
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
Disable the main window toolbar when the modal overlay is shown #30
Conversation
Tested ACK d0cc1f6. Tested on Linux/X11 as much as I could given it's a pretty small change; seems like a nice improvement. I'm not experienced in Qt, but I don't see anything obviously problematic about the code. |
Tested ACK. Tested on Ubuntu 20.04 LTS. It can improve user experience. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK d0cc1f6 tested on Debian 5.7.6-1 (2020-06-24) x86_64 GNU/Linux
Might need a macOS and Windows reviewer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK d0cc1f6 tested on Ubuntu 18.04.4 LTS
I didn't review the code but verified the behavior with and without the PR, and that src/qt/test/test_bitcoin-qt
passes.
Not sure whether it's still helpful but I tested it on MacOS and confirm it worked as well, nicely done @hebasto |
@polylunar thanks! It is very helpful to review before or after a merge. Great to see and looking forward to more tests and review from you 🚀 |
…odal overlay is shown d0cc1f6 qt: Disable toolbar when overlay is shown (Hennadii Stepanov) e74cd20 qt, refactor: Cleanup ModalOverlay slots (Hennadii Stepanov) Pull request description: Keeping the main window toolbar activated while the modal overlay is shown could create the appearance of the non-responsive GUI. Fixes #22. --- On master (ca05588):  With this PR:  ACKs for top commit: harding: Tested ACK d0cc1f6. Tested on Linux/X11 as much as I could given it's a pretty small change; seems like a nice improvement. I'm not experienced in Qt, but I don't see anything obviously problematic about the code. jonatack: ACK d0cc1f6 tested on Debian 5.7.6-1 (2020-06-24) x86_64 GNU/Linux LarryRuane: ACK d0cc1f6 tested on Ubuntu 18.04.4 LTS Tree-SHA512: e371b34231c01e77118deb100e0f280ba1cdef54e317f7f7d6ac322598bda811bd1bfe3035e90d87f8267f4f5d2095d34a8136911159db63694fd1b1b11335a1
Summary: > qt, refactor: Cleanup ModalOverlay slots > qt: Disable toolbar when overlay is shown This is a backport of [[bitcoin-core/gui#30 | core-gui#30]] Test Plan: `bitcoin-qt --regtest Reviewers: #bitcoin_abc, majcosta Reviewed By: #bitcoin_abc, majcosta Differential Revision: https://reviews.bitcoinabc.org/D9994
Keeping the main window toolbar activated while the modal overlay is shown could create the appearance of the non-responsive GUI.
Fixes #22.
On master (ca05588):
With this PR: