backport: bitcoin-core#gui18, #121, #257, #263, #281, #335, #362, #828, bitcoin#21912, #21942, #21988 #6168
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
UdjinM6
requested changes
Aug 2, 2024
src/qt/transactionview.cpp
Outdated
| contextMenu->addAction(abandonAction); | ||
| contextMenu->addAction(resendAction); | ||
| contextMenu->addAction(editLabelAction); | ||
| abandonAction = contextMenu->addAction(tr("Abandon transaction"), this, &TransactionView::abandonTx); |
There was a problem hiding this comment.
gui263:
Suggested change
| abandonAction = contextMenu->addAction(tr("Abandon transaction"), this, &TransactionView::abandonTx); | |
| abandonAction = contextMenu->addAction(tr("Abandon transaction"), this, &TransactionView::abandonTx); | |
| resendAction = contextMenu->addAction(tr("Resend transaction"), this, &TransactionView::resendTx); |
There was a problem hiding this comment.
yes: 4f89c98#diff-dab82be063a09e257636f19825b81c93782c2e3158a4d471a45208a0568edb7cR159 - lost this line in the very first version
… name for QMenu 8233ee4 gui: correct replacement of amp character in the wallet name for QMenu (Konstantin Akimov) Pull request description: In the current implementation Qt uses '&' as a signal to underscore letter and use it as a hot-key, which is not expected for case of wallet name. The [comment in the code](https://github.com/bitcoin/bitcoin/pull/30446/files#diff-2ecf8cbf369cf3d2f3d2b1cf5cfe4c1a647d63e11e2885d2fd0ac11fb5f7a804L402-L404) regarding the use of an "&" on a menu item is misleading. If a wallet name has an "&" in it, it is not supposed to be interpreted as a hot-key, but it should be shown as it is without replacing it to an underscore. See screenshots before & after:   ACKs for top commit: hebasto: re-ACK 8233ee4. pablomartin4btc: tACK 8233ee4 BrandonOdiwuor: ACK 8233ee4. Tested on Ubuntu 22.04 using Qt version 5.15.3 Tree-SHA512: 918c2c05555d203a8b203794c138651d4a1691a05a858631d5a4664b78e150402d1ae4a02ee5181f63a5b22a09badca0a4ea14a626f45f8cbe557226c308b8c5
16c157d qt, refactor: Use better QMenu::addAction overloaded function (Hennadii Stepanov) 7931175 qt: Do not assign Alt+<KEY> shortcuts to context menu actions (Hennadii Stepanov) 963e120 qt: Drop menu separator that separates nothing (Hennadii Stepanov) 1398a65 qt, refactor: Make AddressBookPage::deleteAction a local variable (Hennadii Stepanov) Pull request description: This PR: 1. removes useless `Alt` + `<KEY>` shortcuts from context menu items 2. replaces 3 lines of code with the only call of [`QMenu::addAction`](https://doc.qt.io/qt-5/qmenu.html#addAction-5) for each context menu item (it became possible since bitcoin#21286 was merged) 3. makes other minor cleanups No behavior change. ACKs for top commit: kristapsk: ACK 16c157d promag: Code review ACK 16c157d. Nice code cleanup that takes advantage of more recent Qt API. jarolrod: ACK 16c157d Tree-SHA512: e5555fe957058cc67b351aaf9f09fe3635edb2d07a2223d3093913a25607ae538f0a2fde84c0b0cd43e7475b248949548eb4a5d4b21d8f7391fa2fa8541c04ff
5a4a15d qt, refactor: Drop no longer used PeerTableModel::getRowByNodeId func (Hennadii Stepanov) 9a9f180 qt, refactor: Drop no longer used PeerTableModel::sort function (Hennadii Stepanov) 778a64a qt: Use PeerTableSortProxy for sorting peer table (Hennadii Stepanov) df2d165 qt: Add peertablesortproxy module (Hennadii Stepanov) Pull request description: The "Peers" table in the "Node" window does not hold multiple selection after sorting. This PR introduces a `QSortFilterProxyModel` subclass, that is a standard Qt [practice](https://doc.qt.io/qt-5/model-view-programming.html#custom-sorting-models) for such cases. Now the sorting code is encapsulated into the dedicated Qt class, and we do not need to maintain it. Fixes dashpay#283 (additionally). --- On **master** (7ae86b3): - rows are sorted by "Ping", and a selection is made  - rows are sorted by "NodeId", and the previous selection is _lost_  With **this PR**: - rows are sorted by "Ping", and a selection is made  - rows are sorted by "NodeId", and the row are still selected  ACKs for top commit: jarolrod: re-ACK 5a4a15d, tested on macOS 11.2 Qt 5.15.2 after rebase promag: Tested ACK 5a4a15d. Tree-SHA512: f81c1385892fbf1a46ffb98b42094ca1cc97da52114bbbc94fedb553899b1f18c26a349e186bba6e27922a89426bd61e8bc88b1f7832512dbe211b5f834e076e
… in signal-slot connections cdbc2bd qt: Use template function qOverload in signal-slot connections (Hennadii Stepanov) Pull request description: A nice template function [`qOverload`](https://doc.qt.io/qt-5/qtglobal.html#qOverload) is available for us now (bitcoin#20413, bitcoin#21286). Its usage makes code much more readable. This PR does not change behavior. ACKs for top commit: Talkless: utACK cdbc2bd. promag: Code review ACK cdbc2bd. Tree-SHA512: 72002aa646b1a79bab62d498825b3f245dc7ebdc189280f8bd3b4076e1bb50be8802c02bc872ff6f70c1ea81faec66d3bec36471119dd98c9e70d87b990396ae
07bc22e docs: improve make with parallel jobs description. (Klement Tan) Pull request description: Changed `use -jX here for parallelism` to `use "-j N" for N parallel jobs` **Rationale**: In my opinion `use -jX here for parallelism` is quite ambiguous as it could be perceived as a single option without any argument. Ie running: ```sh make -jX ``` Embarrassingly this caused me to be stuck for quite a long time until I opened the help menu for `make` but if I am the only one who faced this issue I would be happy to close this PR. ACKs for top commit: jarolrod: ACK 07bc22e Tree-SHA512: 2d119b6a461668906c63184b865d2cc9fb2f75abeba34e2e44bc1ef3bcb4adec4a49896ddaf3cc6a20c0095ad20d0de0908401b351eaca9443161d24d6b20d0b
33b0b26 doc: note that brew installed qt is not supported (Raul Siles) Pull request description: picking up bitcoin#21791, the author has stated they [cannot squash](bitcoin#21791 (comment)). This is a useful note to prevent any issues from being opened up about this. The reason that both cannot co-exist and build bitcoin is stated [here](bitcoin#21791 (comment)): > ... the reason is sharing /usr/local/include/ and /usr/local/lib/ directories by both qt5 and qt6 installations. Changes from original PR: - slightly move the note up in this section, this placement seems more appropriate to me - drop "Note:" [PR Render](https://github.com/jarolrod/bitcoin/blob/33b0b26a03a401bd39b88931b69d162c3c538d31/doc/build-osx.md#qt) ACKs for top commit: laanwj: LGTM ACK 33b0b26 hebasto: ACK 33b0b26 Tree-SHA512: f9efac1921a7a33b5791a9f9f4bada4b5369d358fc42e9884c077bfb4dc3f273fdd4432ce012006a8009dfafb87e13bddd56c6336fe84b6133f4b22f849c289a
2a45134 qt: Add shortcuts for console font resize buttons (Hennadii Stepanov) a2e122f qt: Add GUIUtil::AddButtonShortcut (Hennadii Stepanov) 4ee9ee7 qt: Use native presentation of shortcut (Hennadii Stepanov) Pull request description: On `master` the only way to resize the console font is to manually move your mouse and click the resize buttons. This PR introduces convenient keyboard shortcuts to resize the console font. The common resize shortcuts for applications are `Ctrl+=`/`Ctrl++` and `Ctrl+-`/`Ctrl+_`. This means that the resize QPushButtons need two shortcuts each, but you cannot assign multiple shortcuts to a QPushButton. See: https://doc.qt.io/qt-5/qabstractbutton.html#shortcut-prop To get around this, we introduce a new function in `guiutil`, which connects a supplied `QKeySequence` shortcut to a `QAbstractButton`. This function can be reused in other situations where more than one shortcut is needed for a button. | PR on macOS | PR on Linux | | ---------------- | ------------ | |  |  | ACKs for top commit: hebasto: re-ACK 2a45134 Talkless: tACK 2a45134, tested on Debian Sid with Qt 5.15.2, shortcuts still work. Tree-SHA512: e894ccb7e5c695ba83998c21a474d6c587c9c849f12ced665c5e0034feb6b143e41b32ba135cab6cfab22cbf153d5a52b1083b2a278e6dfca3f5ad14c0f6c573
7eea659 qt, test: use qsignalspy instead of qeventloop (Jarol Rodriguez) Pull request description: This PR refactors our GUI `apptests` to use [QSignalSpy](https://doc.qt.io/qt-5/qsignalspy.html) instead of [QEventLoop](https://doc.qt.io/qt-5/qeventloop.html). `QSignalSpy` is more appropriate for our GUI test's as it is purpose-built for testing emission of signals and sets up its own `QEventLoop` when the `wait` function is called. ACKs for top commit: hebasto: ACK 7eea659, tested on Linux Mint 20.1 (Qt 5.12.8). promag: Code review ACK 7eea659. Tree-SHA512: 3adddbcc5efd726302b606980c9923025c44bb8ee16cb8a183e633e423179c0822db66de9ccba20dc5124fff34af4151a379c9cd18130625c60789ce809ee6fd
…on table model cafef08 qt: Refactor to remove unnecessary block in DispatchNotifications (João Barbosa) 57785fb qt: Early subscribe core signals in transaction table model (João Barbosa) c6cbdf1 qt: Refactor ShowProgress to DispatchNotifications (João Barbosa) 3bccd50 qt: Set flag after inital load on transaction table model (João Barbosa) Pull request description: This fixes the case where transaction notifications arrive between `getWalletTxs` and `subscribeToCoreSignals`. Basically notifications are queued until `getWalletTxs` and wallet rescan complete. This is also a requirement to call `getWalletTxs` in a background thread. Motivated by bitcoin#20241. ACKs for top commit: jonatack: tACK cafef08 ryanofsky: Code review ACK cafef08. Only change since last review is splitting commits and replacing m_progress with m_loading. meshcollider: Code review ACK cafef08 Tree-SHA512: 003caab2f2ae3522619711c8d02d521d2b8f7f280a467f6c3d08abf37ca81cc66b4b9fa10acfdf34e5fe250da7b696cfeec435f72b53c1ea97ccda96d8b4be33
|
gui-263 requires follow-up: 3f68f02 | Merge bitcoin-core/gui#362: Add keyboard shortcuts to context menus |
e4c916a Bugfix: GUI: Use a different shortcut for "1 d&ay" banning, due to conflict with "&Disconnect" (Luke Dashjr) 94e7cdd GUI: Add keyboard shortcuts for other context menus (Luke Dashjr) 02b5263 GUI: Restore keyboard shortcuts for context menu entries (Luke Dashjr) Pull request description: Various keyboard shortcuts were lost in dashpay#263; this restores them, and also adds new ones for other context menus. Note that with a context menu open, simply the shortcut by itself (no Alt) is used. ACKs for top commit: jarolrod: Code Review ACK e4c916a hebasto: ACK e4c916a, tested on Linux Mint 20.1 (Qt 5.12.8). Tree-SHA512: 949461acf7aac592bc48a1c5abad41b167365830e0cedb3aa11b6a87bd347e16126830ea87936f9c9efc4b7df5b09d3833fae784964d6d119ed45703cfba2ffd
knst
changed the title
src/qt/governancelist.cpp
Outdated
Comment on lines
390
to
394
| QString proposal_url = proposal->url(); | ||
| proposal_url.replace(QChar('&'), QString("&&")); | ||
|
|
||
| // right click menu with option to open proposal url | ||
| QAction* openProposalUrl = new QAction(proposal->url(), this); | ||
| QAction* openProposalUrl = new QAction(proposal_url, this); |
There was a problem hiding this comment.
I don't understand the purpose of c36bb8e
There was a problem hiding this comment.
same as bitcoin-core/gui#828
(see screenshot)
src/qt/governancelist.cpp
Outdated
Comment on lines
390
to
395
| // right click menu with option to open proposal url | ||
| QString proposal_url = proposal->url(); | ||
| proposal_url.replace(QChar('&'), QString("&&")); | ||
|
|
||
| // right click menu with option to open proposal url | ||
| QAction* openProposalUrl = new QAction(proposal_url, this); | ||
| proposalContextMenu->clear(); | ||
| proposalContextMenu->addAction(openProposalUrl); | ||
| connect(openProposalUrl, &QAction::triggered, proposal, &Proposal::openUrl); | ||
| proposalContextMenu->addAction(proposal_url, proposal, &Proposal::openUrl); |
There was a problem hiding this comment.
please edit the commit message for 1cdd9fb to include justification / why
There was a problem hiding this comment.
same as bitcoin-core/gui#263
- replaces 3 lines of code with the only call of
QMenu::addActionfor each context menu item (it became possible since build: Bump minimum Qt version to 5.9.5 bitcoin/bitcoin#21286 was merged)
UdjinM6
approved these changes
Aug 21, 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
Just regular backports from bitcoin v22, mostly Qt related
What was done?
See commits for a list of backports.
This PR also fixes a rendering an url on Governance tab if it has any '&' inside. see screenshots.
Original url:
https://example.com/?test=nothing&to=see&&lol- yes, double '&&' just for test even if url is silly.Failed behaviour:
Correctly rendered:
How Has This Been Tested?
Run unit/functional tests
Breaking Changes
N/A
Checklist: