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

pull hebasto wants to merge 1 commits into bitcoin-core:master from hebasto:210323-peers changing 2 files +26 −6
  1. hebasto commented at 11:24 pm on March 23, 2021: member
  2. hebasto added the label Feature on Mar 23, 2021
  3. jarolrod commented at 2:47 am on March 26, 2021: member

    ACK dda2f65d754aa05d8191e79e4db2189e79988443, tested on macOS 11.2.3 Qt 5.15.2

    This PR successfully saves/restores the column sizes in the peers tab. Below are some screenshots showing the behavior:

    Start Screen Shot 2021-03-25 at 10 42 18 PM

    Resize Screen Shot 2021-03-25 at 10 42 42 PM

    Close and Restart Screen Shot 2021-03-25 at 10 43 05 PM

  4. jonatack approved
  5. jonatack commented at 4:26 pm on March 26, 2021: contributor

    Very practical improvement.

    ACK dda2f65d754aa05d8191e79e4db2189e79988443 code review, debug build is clean, tested repeatedly with both “Peers” and “Banned Peers” column widths and different shutdown methods. Tested with Qt 5.15.2 on Debian 5.10.19-1 (2021-03-02).

    Changes are straightforward. I’m not a Qt expert so I briefly checked the Qt 5.15 QByteArray, saveState and restoreState docs and version compats.

  6. hebasto closed this on Mar 28, 2021
  7. hebasto reopened this on Mar 28, 2021
  8. hebasto closed this on Mar 28, 2021
  9. hebasto reopened this on Mar 28, 2021
  10. hebasto commented at 6:18 pm on April 20, 2021: member

    @promag

    Mind looking into this PR?

  11. in src/qt/rpcconsole.cpp:1186 in dda2f65d75 outdated 
    1197@@ -1186,6 +1198,11 @@ void RPCConsole::showEvent(QShowEvent *event)
1198 
1199 void RPCConsole::hideEvent(QHideEvent *event)
1200 {
1201+    // It is too late to call QHeaderView::saveState() in ~RPCConsole(), as all of
    promag commented at 3:19 pm on April 23, 2021:
    An alternative could be to use QHeaderView::sectionResized signal.
    hebasto commented at 4:54 pm on April 23, 2021:

    Yes, using QHeaderView::sectionResized signal is a clearer solution. But most of them are redundant, as we are interested in the final state only.

    Do you think we should prefer clear code (I admit that using a comment to explain code is a bad sign)? If so, I’ll be happy to implement your suggestion.

    promag commented at 4:58 pm on April 23, 2021:

    On the other hand with current approach it always save even if you don’t resize.

    Is the signal emitted while resizing? Not only at the end? (Didn’t test)

    hebasto commented at 1:37 pm on April 28, 2021:

    @promag

    Is the signal emitted while resizing? Not only at the end? (Didn’t test)

    The signal is emitted while resizing, at least on Linux Mint 20.1 + Qt 5.12.8.

  12. in src/qt/rpcconsole.cpp:620 in dda2f65d75 outdated 
    614@@ -612,9 +615,14 @@ void RPCConsole::setClientModel(ClientModel *model, int bestblock_height, int64_
615         ui->peerWidget->setSelectionBehavior(QAbstractItemView::SelectRows);
616         ui->peerWidget->setSelectionMode(QAbstractItemView::ExtendedSelection);
617         ui->peerWidget->setContextMenuPolicy(Qt::CustomContextMenu);
618-        ui->peerWidget->setColumnWidth(PeerTableModel::Address, ADDRESS_COLUMN_WIDTH);
619-        ui->peerWidget->setColumnWidth(PeerTableModel::Subversion, SUBVERSION_COLUMN_WIDTH);
620-        ui->peerWidget->setColumnWidth(PeerTableModel::Ping, PING_COLUMN_WIDTH);
621+
622+        QSettings settings;
623+        m_peer_widget_header_state = settings.value("PeersTabPeerHeaderState").toByteArray();
    promag commented at 3:20 pm on April 23, 2021:
    Load m_peer_widget_header_state in constructor where settings already exists.
    hebasto commented at 3:56 pm on April 23, 2021:

    @promag

    Thanks for raising this question!

    From my experience QTableView::setColumnWidth does not work without model set (but it is not documented by Qt).

    Did I miss something? Does it work for you?

    promag commented at 3:58 pm on April 23, 2021:
    I mean move L620 to constructor but leave restoreState() here.
    hebasto commented at 7:49 pm on April 29, 2021:
    Thanks! Updated.
  13. promag commented at 3:20 pm on April 23, 2021: contributor
    Tested ACK dda2f65d754aa05d8191e79e4db2189e79988443.
  14. hebasto force-pushed on Apr 29, 2021
  15. hebasto commented at 7:49 pm on April 29, 2021: member

    Updated dda2f65d754aa05d8191e79e4db2189e79988443 -> 002d0e0d998d4c15d86990f9ce41d981e0bad7fc (pr256.01 -> pr256.02, diff):

  16. promag commented at 8:01 am on May 5, 2021: contributor
    Code review ACK 002d0e0d998d4c15d86990f9ce41d981e0bad7fc.
  17. hebasto added this to the milestone 22.0 on May 8, 2021
  18. hebasto commented at 7:04 am on May 8, 2021: member

    @jonatack @jarolrod

    Do you mind re-reviewing this PR?

  19. hebasto force-pushed on May 10, 2021
  20. hebasto commented at 10:40 pm on May 10, 2021: member
    Rebased 002d0e0d998d4c15d86990f9ce41d981e0bad7fc -> 03370446f7fc4936596c7b0d689ab425bdbc825d (pr256.02 -> pr256.03) due to the conflict with #194.
  21. jarolrod commented at 2:45 am on May 13, 2021: member

    ACK 03370446f7fc4936596c7b0d689ab425bdbc825d

    patch looks good

    Start Resize Restart
    Screen Shot 2021-05-12 at 10 41 14 PM Screen Shot 2021-05-12 at 10 41 40 PM Screen Shot 2021-05-12 at 10 43 35 PM
  22. hebasto referenced this in commit 5c041cb348 on May 26, 2021
  23. hebasto commented at 12:28 pm on May 29, 2021: member

    @jonatack @promag

    Hoping on confirming your ACKs after the recent rebasing :)

  24. DrahtBot added the label Needs rebase on Jun 1, 2021
  25. qt: Save/restore column sizes of the tables in the Peers tab fb1b1e0f3e
  26. hebasto force-pushed on Jun 5, 2021
  27. hebasto commented at 10:40 am on June 5, 2021: member
    Rebased 03370446f7fc4936596c7b0d689ab425bdbc825d -> fb1b1e0f3eae32b087347889ed7502b7f2c48549 (pr256.03 -> pr256.04) due to the conflict with #123.
  28. DrahtBot removed the label Needs rebase on Jun 5, 2021
  29. jonatack commented at 1:27 pm on June 5, 2021: contributor

    This is a real UX improvement, thanks!

    ACK fb1b1e0f3eae32b087347889ed7502b7f2c48549 code review, debug-built and tested

    I am seeing some unusual flickering in the User Agent header, but this is also the case in current master 38ab7d0765e52b8. I don’t recall seeing it before but am not sure.

  30. hebasto commented at 2:36 pm on June 5, 2021: member

    @jonatack

    I am seeing some unusual flickering in the User Agent header, but this is also the case in current master 38ab7d0. I don’t recall seeing it before but am not sure.

    Reported in #191, fixed in #164.

  31. jarolrod commented at 7:21 pm on June 5, 2021: member

    ACK fb1b1e0f3eae32b087347889ed7502b7f2c48549

    Code Review ACK and Tested ACK on Linux Qt 5.15.2, macOS 11.3 Qt 5.15.2, and cross-compile to Windows

    Arch Linux:

    Start Resize Restart
    before-resize after-resize restart

    macOS:

    Start Resize Restart
    before-resize after-resize restart

    Windows:

    Start Resize Restart
    before-resize after-resize restart
  32. hebasto merged this on Jun 5, 2021
  33. hebasto closed this on Jun 5, 2021
  34. hebasto deleted the branch on Jun 5, 2021
  35. sidhujag referenced this in commit 97504980a7 on Jun 6, 2021
  36. gwillen referenced this in commit 9e27d6c746 on Jun 1, 2022
  37. bitcoin-core locked this on Aug 16, 2022


hebasto jarolrod jonatack promag

Labels
Feature

Milestone
22.0

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/gui. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-22 02:20 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me