Save/restore TransactionView and recentRequestsView tables column sizes

hebasto commented at 4:14 pm on January 31, 2021: member

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

Based on #204 (the first commit).

qt: Drop buggy TableViewLastColumnResizingFixer class

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.

3913d1e8c1

qt, refactor: Drop intermediate assignment

This change improves code readability.

ecdbaf71c0

qt: Move transactionView properties settings to constructor

This is move-only change.

788205c3f7

qt: Save/restore TransactionView table column sizes

Sorting order is not saved/restored intentionally.

9c5f4f2169

qt: Move recentRequestsView properties settings to constructor

This is move-only change.

f5c8093e77

qt: Save/restore recentRequestsView table column sizes

Sorting order is not saved/restored intentionally.

964885d048

jonatack commented at 7:50 pm on January 31, 2021: contributor

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 commented at 7:59 pm on January 31, 2021: member

… 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 commented at 3:09 pm on February 1, 2021: contributor

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

jonasschnelli commented at 7:28 am on February 4, 2021: contributor

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

hebasto commented at 12:20 pm on February 4, 2021: member

@jonasschnelli

I guess this is also resettable by -resetguisettings?

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

jarolrod commented at 6:37 am on February 8, 2021: member

ACK 964885d04801c6ab77ce4705cff01c9d83bc3ed8, 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

in src/qt/transactionview.cpp:154 in 964885d048

161+    transactionView->setSelectionMode(QAbstractItemView::ExtendedSelection);
162+    transactionView->setSortingEnabled(true);
163+    transactionView->verticalHeader()->hide();
164+
165+    QSettings settings;
166+    if (!transactionView->horizontalHeader()->restoreState(settings.value("TransactionViewHeaderState").toByteArray())) {

Talkless commented at 8:44 pm on February 20, 2021:

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?

hebasto commented at 7:56 am on February 21, 2021:

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.

Talkless commented at 11:44 am on February 21, 2021:

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().

Talkless approved

Talkless commented at 8:48 pm on February 20, 2021: none

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

MarcoFalke referenced this in commit fd725c2d79 on Feb 22, 2021

MarcoFalke merged this on Feb 22, 2021

MarcoFalke closed this on Feb 22, 2021

hebasto deleted the branch on Feb 22, 2021

sidhujag referenced this in commit c31b293206 on Feb 22, 2021

Geremia commented at 3:38 am on February 27, 2021: none

This fixed the [column widths issue #21306](https://github.com/bitcoin/bitcoin/issues/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 commented at 4:06 pm on February 27, 2021: member

@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 commented at 7:32 pm on February 27, 2021: none

@hebasto

Are you sure about the “Requested payments history” tab?

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

MarcoFalke referenced this in commit 63314b8211 on Mar 10, 2021

hebasto commented at 0:26 am on March 26, 2021: member

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).

:disappointed:

gwillen referenced this in commit 41743b65e8 on Jun 28, 2022

bitcoin-core locked this on Aug 16, 2022

Save/restore TransactionView and recentRequestsView tables column sizes #205