backport: bitcoin-core/gui#29, gui#123, #164, gui#256, gui#309, gui#313, gui#329, gui#331, gui#333, gui#346, gui#393 #6285
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
knst
changed the title
knst
changed the title
a02c970 qt, refactor: Revert explicit including QStringBuilder (Hennadii Stepanov) 3fd3a0f qt, build: Optimize string concatenation (Hennadii Stepanov) Pull request description: From [Qt docs](https://doc.qt.io/qt-5/qstring.html#more-efficient-string-construction): > ... multiple uses of the \[`QString`\] '+' operator usually means multiple memory allocations. When concatenating n substrings, where n > 2, there can be as many as n - 1 calls to the memory allocator. With this PR > ... the '+' will automatically be performed as the `QStringBuilder` '%' everywhere. The change in the `src/Makefile.qt.include` file does not justify submitting this PR into the main repo, IMHO. ACKs for top commit: laanwj: Code review ACK a02c970 Talkless: utACK a02c970, built successfully on Debian Sid with Qt 5.15.2, but did not check if any displayed strings are "wrong" after refactoring. jarolrod: ACK a02c970 Tree-SHA512: cbb476ee96f27c3bd6e125efab74d8bf24bbdb4c30576b3feea45e203405f3bf5b497dd7d3e11361fc825fcbf4b893b152921a9efdeaf73b42d1865d85f0ae84
knst
changed the title
knst
force-pushed
the
bp-v22-p19
branch
2 times, most recently
from
3a5dc19 to
0ca2c43
Compare
knst
changed the title
|
It seems as all GUI changes in this PR doesn't really worth to be mentioned in the Release notes, maybe except quick access for "online/offline" switch from peer button, but I am open for suggestions note: gui#393 (fix regression in "Encrypt Wallet") is actually fix for gui#29 which is in this PR. So, no bug has been ever introduced for us - no need release-notes. |
thephez
reviewed
Sep 25, 2024
That's right, we have our own style by "css", so, these changes in backport doesn't actually changed style of buttons, just unified code with bitcoin. They doesn't look nice as rectangle buttons here I think, should we change it? |
I don't mind them the way they are. But I'm a horrible judge of good ui/ux 😄 |
|
LGTM but gui346 shouldn't be partial imo - we update translations all at once so there won't be another |
df4c81f English translations update (Hennadii Stepanov) bfb53dd scripted-diff: Fix ellipsis after pr20773 (Hennadii Stepanov) Pull request description: Update for Transifex. After changing translator comments in dashpay#332 this update will show if Transifex triggers strings to be re-translated. ACKs for top commit: laanwj: ACK df4c81f jarolrod: ACK df4c81f Tree-SHA512: 1e54812bc04db6ae39e0b4d735b220ed8730a9941b17a0a2d09e21bcdd08e829adba86c35cf43c9be5e492ccb13e53a90149fcd7d6c0f5fdd022b978a1ff785c
…ork icon d29ea72 gui: Add access to the Peers tab from the network icon (Hennadii Stepanov) Pull request description: This PR add a small context menu to the network activity icon that provides an access to the Peers tab:  Closes dashpay#93. ACKs for top commit: Sjors: re-ACK d29ea72 kristapsk: re-ACK d29ea72 promag: Code review ACK d29ea72. Tree-SHA512: dd871415fe514a19c6a22100d58f31954d9e55b80585d5a3f26e17a8d51dadf912441786fc0d23beabd812f1b501658fec1dbe345cd41beae5832a8eda890f77
…ion-friendly 0f3d955 qt: Make RPC console welcome message translation-friendly (Hennadii Stepanov) Pull request description: The best practice is do not split a translatable multi-line message into single lines. This helps translators to follow the context. ACKs for top commit: jarolrod: re-ACK 0f3d955 Tree-SHA512: 30911ff3a972a7787804bb8b27d0b77bfff15939bb478c199261866bfb55d9acd12ab4d44b8b9fc1d4898222cabc4007cc897f9b65728924d121f31e914c44ac
…g another one 38eb37c qt, rpc: Do not accept command while executing another one (Hennadii Stepanov) 0c32b9c qt, rpc: Accept stop RPC even another command is executing (Hennadii Stepanov) ccf7902 qt, rpc, refactor: Return early in RPCConsole::on_lineEdit_returnPressed (Hennadii Stepanov) 5b9c8c9 qt, rpc: Add "Executing…" message (Hennadii Stepanov) Pull request description: On master (3f512f3) it is possible to enter another command while the current command is still being executed. That makes a mess in the output. With this PR:  Some previous context: bitcoin-core/gui#59 (comment) --- It is still possible to enter and execute the `stop` command any time. ACKs for top commit: jarolrod: ACK 38eb37c promag: Tested ACK 38eb37c. Tree-SHA512: 2b37a4b6838bf586b1b5c878192106721f713caeb6252514a6540356aab898986396e0777e73891d331b1be797a4926c20d3f9f38ba2c984ea90d55b0c34f664
8b419b5 qt: make console buttons look clickable (Jarol Rodriguez) Pull request description: On master, for macOS, the console buttons' hitboxes are quite small. This makes clicking on the button with your mouse a little more tedious than it should be. The Issue is related to recent versions of Qt (>5.9.8) not playing so nice on macOS when there are "incorrect" `width` and `height` values set for a `QPushButton` (here is another example: bitcoin-core/gui#319 (review)). This fixes this small hitbox issue by converting the buttons from `QPushButton` to `QToolButton`, which in turn makes the buttons look explicitly clickable. This approach was chosen as it helps us avoid having to play around with `width` and `height` values until we find values that play nice with macOS and look good on Linux & Windows. Also, `QToolButton` is an appropriate class for these buttons. Per [Qt Docs](https://doc.qt.io/qt-5/qtoolbutton.html#details): > A tool button is a special button that provides quick-access to specific commands or options. As opposed to a normal command button, a tool button usually doesn't show a text label, but shows an icon instead. Since we are changing the type of the buttons, we need to change the respective actions connection logic in `rpcconsole`. Instead of plugging in `QToolButton`, we abstract it to the base class: `QAbstractButton`. per [Qt Dev Notes](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Developer-Notes-for-Qt-Code#inherited-signals-and-slot) > Use base class functions as this makes the code more general, e.g., use QAbstractButton::clicked instead of QPushButton::clicked. While here, we also update the size of the icons to `22x22` to be consistent with other tool buttons. **macOS: Master vs PR:** | Master | PR | | ----------- | ----------- | |  |  | **Linux: Master vs PR:** | Master | PR | | ----------- | ----------- | |  |  | ACKs for top commit: hebasto: ACK 8b419b5, tested on Linux Mint 20.1 (Qt 5.12.8). promag: Tested ACK 8b419b5 on macOS Big Sur M1, this drops only relevant usages to `flat` buttons. Tree-SHA512: 3f3cdcbe83398136a1d1ee8fc2835be8681f2ed39e79db1e939cab6a00a779f528343d54992807a845cc84d9ef13591affb7a6dbca9e5753a2b8665b0af4d611
…n the Peers tab fb1b1e0 qt: Save/restore column sizes of the tables in the Peers tab (Hennadii Stepanov) Pull request description: ACKs for top commit: jonatack: ACK fb1b1e0 code review, debug-built and tested jarolrod: ACK fb1b1e0 Tree-SHA512: f93495ecd13e4202aba61b407fffbeec855f5b0c1cc027197c78edddd7d11c87ebdb0fcb1daac242f0407323b31f4e7e0313bd76113a5241e4c868a8829af20a
… logic 62cb8d9 qt: Drop BitcoinGUI* WalletFrame data member (Hennadii Stepanov) f73e5c9 qt: Move CreateWalletActivity connection from WalletFrame to BitcoinGUI (Hennadii Stepanov) 20e2e24 qt: Move WalletView connections from WalletFrame to BitcoinGUI (Hennadii Stepanov) Pull request description: This PR: - implements an idea from bitcoin#17937 (comment) - simplifies `WalletFrame` class interface - as a side effect, removes `bitcoingui` -> `walletframe` -> `bitcoingui` circular dependency - is an alternative to bitcoin#17500 ACKs for top commit: promag: Tested ACK 62cb8d9 on macos 11.2.3 with depends build. jarolrod: ACK 62cb8d9 Tree-SHA512: 633b526a8499ba9ab4b16928daf4de4f6d610284bb9fa51891cad35300a03bde740df3466a71b46e87a62121330fcc9e606eac7666ea5e45fa6d5785b60dcbbd
ecbd911 qt: Handle peer addition/removal in a right way (Hennadii Stepanov) 1b66f6e qt: Drop PeerTablePriv class (Hennadii Stepanov) efb7e5a qt, refactor: Use default arguments for overridden functions (Hennadii Stepanov) Pull request description: This PR makes `PeerTableModel` handle a peer addition/removal in a right way. See: - https://doc.qt.io/qt-5/model-view-programming.html#inserting-and-removing-rows - https://doc.qt.io/qt-5/model-view-programming.html#resizable-models Fixes dashpay#160. Fixes dashpay#191. ACKs for top commit: jarolrod: re-ACK ecbd911 promag: reACK ecbd911 just improvements to the comment since last review. Tree-SHA512: 074935d67f78561724218e8b33822e2de16749f873c29054926b720ffcd642f08249a222b563983cf65a9b716290aa14e2372c47fc04e5f401f759db25ca710f
f507681 qt: Connect WalletView signal to BitcoinGUI slot directly (Hennadii Stepanov) bd50ff9 qt: Drop redundant OverviewPage::handleOutOfSyncWarningClicks slot (Hennadii Stepanov) 793f195 qt: Drop redundant WalletView::requestedSyncWarningInfo slot (Hennadii Stepanov) Pull request description: This PR: - removes slots whose only job is to emit a signal, since we can use the signal as a slot - connects the`WalletView::outOfSyncWarningClicked` signal to the `BitcoinGUI::showModalOverlay` slot directly, and removes intermediate `WalletFrame` slot and signal - split from dashpay#29 This PR does not change behavior. ACKs for top commit: Talkless: tACK f507681, tested on Debian Sid with Qt 5.15.2, no any behavioral changes noticed. promag: Code review ACK f507681. Tree-SHA512: cd636a7e61881b2cbee84d5425d2107a8e39683b8eb32d79dc9ea942db55d5c1979be2f70da1660eaee5de622d10ed5a92f11fc2351de21b84324b10b23d0c96
d54d949 qt: Fix regression in "Encrypt Wallet" menu item (Hennadii Stepanov) Pull request description: Fix dashpay#392. Adding a new item to the `m_wallet_selector` must follow the establishment of a connection between the `WalletView::encryptionStatusChanged` signal and the `BitcoinGUI::updateWalletStatus` slot. This was a regression introduced in bitcoin@20e2e24 (dashpay#29). --- An _encrypted_ wallet being auto-loaded at the GUI startup: - on master (eaf09bd)  - with this PR  ACKs for top commit: achow101: ACK d54d949 jarolrod: ACK d54d949 Tree-SHA512: 669615ec8e1517c2f4cdf59bd11a7c85be793ba0dda112361cf95e6c2f0636215fed331d26a86dc9b779a49defae1b248232f98dab449584376c111c288e87bb
Indeed, removed "partial" mark |
knst
changed the title
UdjinM6
approved these changes
Sep 26, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
Gui related backports from bitcoin v22
What was done?
See commits
How Has This Been Tested?
Run unit/functional tests
See also:
Breaking Changes
N/A
Checklist: