Save/restore TransactionView and recentRequestsView tables column sizes #205

hebasto · 2021-01-31T16:14:41Z

Save/restore TransactionView and recentRequestsView tables column sizes.
Sorting order is not saved/restored intentionally.

Based on #204 (the first commit).

In Qt 5 the last column resizing with dragging its left edge works out-of-the-box. The current TableViewLastColumnResizingFixer implementation could put the last column content out of the view port and confuse a user.

This change improves code readability.

This is move-only change.

Sorting order is not saved/restored intentionally.

This is move-only change.

Sorting order is not saved/restored intentionally.

jonatack · 2021-01-31T19:50:03Z

Concept ACK if the idea is to save the column sizes if adjusted by the user and restore them on next use (is that the goal of this pull?), ATM for me on Debian the Transactions window column widths are indeed not persisted after shutdown. (Why not save the sorting order too?)

hebasto · 2021-01-31T19:59:20Z

... if the idea is to save the column sizes if adjusted by the user and restore them on next use (is that the goal of this pull?)...

Correct.

Why not save the sorting order too?

After a while users could forget/unnotice another sort order, and that could cause their panic (when they are not looking at the recently made transaction).

EDIT: while testing the sorting order preservation was uncomfortable for me.

jonatack · 2021-02-01T15:09:25Z

Tested, it seems to work well. Sort is not preserved but column widths are, as described.

jonasschnelli · 2021-02-04T07:28:04Z

Concept ACK (I guess this is also resettable by -resetguisettings?)

hebasto · 2021-02-04T12:20:39Z

@jonasschnelli

I guess this is also resettable by -resetguisettings?

Correct. -resetguisettings affects all of the settings those use a QSettings class instance.

jarolrod

ACK 964885d, tested on macOS 11.1 Qt 5.15.2

PR works well and does what it says it will do. Confirmed that the column width's of transactions and recent requests are saved and then restored upon node restart. Code looks good and it removes some complexity (yay, less lines of code).

Below are screenshots comparing the behavior between master and this pr

Master: Start point

Master: Resize

Master: Restart, column width does not persist

PR: Start

PR: Resize

PR: Restart, column width persists

Talkless

tACK 964885d, tested on Debian Sid, saving/restoring and resetting (with -resetguisettings) works as expected.

Talkless · 2021-02-20T20:44:50Z

src/qt/transactionview.cpp

+    transactionView->verticalHeader()->hide();
+
+    QSettings settings;
+    if (!transactionView->horizontalHeader()->restoreState(settings.value("TransactionViewHeaderState").toByteArray())) {


tiny nit:
TransactionViewHeaderState string literal is used twice in same .cpp file. Maybe we can then consider this as "magic string" (as magic constant) and avoid duplicating, using some local .cpp-file constant?

This is the current way to work with setting names in the codebase.
Your suggestion is nice to apply to all of the setting names in a follow up pr.

Your suggestion is nice to apply to all of the setting names in a follow up pr.

True. Maybe even GUISettings class to wrap calls to QSettings with methods like getTransactionViewHeaderState().

3913d1e qt: Drop buggy TableViewLastColumnResizingFixer class (Hennadii Stepanov) Pull request description: In Qt 5 the last column resizing with dragging its left edge works out-of-the-box. The current `TableViewLastColumnResizingFixer` implementation could put the last column content out of the view port and confuse a user: ![Screenshot from 2021-01-31 18-04-32](https://user-images.githubusercontent.com/32963518/106390022-fd6bd180-63ee-11eb-9216-6e5117f8dc96.png) Historical context: - bitcoin/bitcoin#2862 - bitcoin/bitcoin#3626 - bitcoin/bitcoin#3738 - bitcoin/bitcoin#3920 #205 is a nice addition. ACKs for top commit: jarolrod: ACK 3913d1e, tested on macOS 11.1 Qt 5.15.2 Talkless: tACK 3913d1e, tested on Debian Sid. Can confirm that behavior in previous commit does not produce scroll bar, last column gets "hidden". This PR makes clear that there's more to see in the view. promag: Tested ACK 3913d1e on macos. Tree-SHA512: 12582dfce54bb1db3d9934ae092e305d32e9760cc99b0265322e161fa7f54b7d6fb6cefedf700783f767d5c3a56a8545c8d2f5ade66596c4e67b8a5287063e8a

…questsView tables column sizes 964885d qt: Save/restore recentRequestsView table column sizes (Hennadii Stepanov) f5c8093 qt: Move recentRequestsView properties settings to constructor (Hennadii Stepanov) 9c5f4f2 qt: Save/restore TransactionView table column sizes (Hennadii Stepanov) 788205c qt: Move transactionView properties settings to constructor (Hennadii Stepanov) ecdbaf7 qt, refactor: Drop intermediate assignment (Hennadii Stepanov) Pull request description: Save/restore TransactionView and recentRequestsView tables column sizes. Sorting order is not saved/restored intentionally. Based on #204 (the first commit). ACKs for top commit: jarolrod: ACK 964885d, tested on macOS 11.1 Qt 5.15.2 Talkless: tACK 964885d, tested on Debian Sid, saving/restoring and resetting (with `-resetguisettings`) works as expected. Tree-SHA512: c24e41bf4d95bb33dce16e9a0b952ffd0912e95f4d2a1bc5292fcf5a27100e70fea73433c4ff246d05b174fc23a7b6de1790a2e8b990a9089e4deca79a00dedc

Geremia · 2021-02-27T03:38:27Z

This fixed the column widths issue #21306; but now, after opening bitcoin-qt, the transactions aren't chronologically sorted until I click the date column header twice.
Also, this doesn't fix "Requested payments history" and "Peers" column widths issues.

hebasto · 2021-02-27T16:06:25Z

@Geremia

but now, after opening bitcoin-qt, the transactions aren't chronologically sorted until I click the date column header twice.

Thank you for reporting. Fixed in #229.

Also, this doesn't fix "Requested payments history" and "Peers" column widths issues.

Are you sure about the "Requested payments history" tab?

The "Peers" tab is WIP :)

Geremia · 2021-02-27T19:32:53Z

@hebasto

Are you sure about the "Requested payments history" tab?

It remembers the widths there now.
I applied #229, too.

@leonardojobim

c524dc5 qt: Fix regression with initial sorting after pr205 (Hennadii Stepanov) Pull request description: Unfortunately, #205 introduced a regression. After opening the "Receive" or "Transaction" tab at first time despite of the "Date" header is marked as sorted, table rows are not sorted actually: ![Screenshot from 2021-02-27 17-49-54](https://user-images.githubusercontent.com/32963518/109392491-f7e9a480-7924-11eb-96cc-98b6f932e18e.png) It appears that sorting the table must be triggered _after_ the `QTableView::setModel` call. With this PR (and pre-#205): ![Screenshot from 2021-02-27 17-48-40](https://user-images.githubusercontent.com/32963518/109392505-08018400-7925-11eb-8107-8f8685744b83.png) ACKs for top commit: Talkless: tACK c524dc5, tested on Debian Sid with Qt 5.15.2. I can confirm @leonardojobim observations. leonardojobim: Tested ACK c524dc5 on Ubuntu 20.04.2 Qt 5.12.8 jonatack: ACK c524dc5 jarolrod: ACK c524dc5, tested on macOS 11.1 Qt 5.15.2 Tree-SHA512: e370229979a70d63a0b64dbc11c4eca338695a070881d4d8f015644617f180e6accc24d6bdf98a75e7c9ba9be2a0ace9a2b7eb9c783ebb2992c3b2c3b3deb408

hebasto · 2021-03-26T00:26:53Z

While working on #256, two flaws were noted in this PR.

In multi-wallet environment settings of the last closed wallet tables are saved only.
QTableView::setColumnWidth does not work without model set (but it is not documented by Qt).

😞

Some commented-out GUI code becomes conditional ... maybe it doesn't need to be commented out anymore? I dunno, I left it alone.

hebasto added 6 commits January 31, 2021 18:09

qt: Drop buggy TableViewLastColumnResizingFixer class

3913d1e

In Qt 5 the last column resizing with dragging its left edge works out-of-the-box. The current TableViewLastColumnResizingFixer implementation could put the last column content out of the view port and confuse a user.

qt, refactor: Drop intermediate assignment

ecdbaf7

This change improves code readability.

qt: Move transactionView properties settings to constructor

788205c

This is move-only change.

qt: Save/restore TransactionView table column sizes

9c5f4f2

Sorting order is not saved/restored intentionally.

qt: Move recentRequestsView properties settings to constructor

f5c8093

This is move-only change.

qt: Save/restore recentRequestsView table column sizes

964885d

Sorting order is not saved/restored intentionally.

hebasto mentioned this pull request Jan 31, 2021

Drop buggy TableViewLastColumnResizingFixer class #204

Merged

jarolrod reviewed Feb 8, 2021

View reviewed changes

Talkless approved these changes Feb 20, 2021

View reviewed changes

maflcko merged commit 0e9596c into bitcoin-core:master Feb 22, 2021

hebasto deleted the 210131-header branch February 22, 2021 07:23

jonatack mentioned this pull request Feb 27, 2021

Transactions columns widths not remembered after quit bitcoin/bitcoin#21306

Closed

hebasto mentioned this pull request Feb 27, 2021

Fix regression with initial sorting after pr205 #229

Merged

This was referenced Apr 23, 2021

Save/restore column sizes of the tables in the Peers tab #256

Merged

doc: Naming convention for new QSettings setting names #295

Open

jarolrod mentioned this pull request May 24, 2021

qt: peers-tab resizeColumnsToContents #338

Closed

luke-jr mentioned this pull request Jun 24, 2021

Bugfix: When restoring table columns, still set their minimum column width and stretch on last section #368

Closed

gwillen mentioned this pull request Mar 30, 2022

Bring Elements up to date with Bitcoin Core 22.0 ElementsProject/elements#1058

Closed

bitcoin-core locked as resolved and limited conversation to collaborators Aug 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Save/restore TransactionView and recentRequestsView tables column sizes #205

Save/restore TransactionView and recentRequestsView tables column sizes #205

hebasto commented Jan 31, 2021 •

edited

Loading

jonatack commented Jan 31, 2021

hebasto commented Jan 31, 2021 •

edited

Loading

jonatack commented Feb 1, 2021

jonasschnelli commented Feb 4, 2021

hebasto commented Feb 4, 2021

jarolrod left a comment

Talkless left a comment

Talkless Feb 20, 2021

hebasto Feb 21, 2021

Talkless Feb 21, 2021

Geremia commented Feb 27, 2021 •

edited

Loading

hebasto commented Feb 27, 2021

Geremia commented Feb 27, 2021 •

edited

Loading

hebasto commented Mar 26, 2021 •

edited

Loading

Save/restore TransactionView and recentRequestsView tables column sizes #205

Save/restore TransactionView and recentRequestsView tables column sizes #205

Conversation

hebasto commented Jan 31, 2021 • edited Loading

jonatack commented Jan 31, 2021

hebasto commented Jan 31, 2021 • edited Loading

jonatack commented Feb 1, 2021

jonasschnelli commented Feb 4, 2021

hebasto commented Feb 4, 2021

jarolrod left a comment

Choose a reason for hiding this comment

Talkless left a comment

Choose a reason for hiding this comment

Talkless Feb 20, 2021

Choose a reason for hiding this comment

hebasto Feb 21, 2021

Choose a reason for hiding this comment

Talkless Feb 21, 2021

Choose a reason for hiding this comment

Geremia commented Feb 27, 2021 • edited Loading

hebasto commented Feb 27, 2021

Geremia commented Feb 27, 2021 • edited Loading

hebasto commented Mar 26, 2021 • edited Loading

hebasto commented Jan 31, 2021 •

edited

Loading

hebasto commented Jan 31, 2021 •

edited

Loading

Geremia commented Feb 27, 2021 •

edited

Loading

Geremia commented Feb 27, 2021 •

edited

Loading

hebasto commented Mar 26, 2021 •

edited

Loading