Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emit dataChanged signal to dynamically re-sort Peers table #375

Merged
merged 1 commit into from
Jul 5, 2021

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Jun 28, 2021

By default, the PeerTableSortProxy

dynamically re-sorts ... data whenever the original model changes.

That is not the case on master (8cdf917) as in ecbd911 (#164) no signals are emitted to notify about model changes.

This PR uses a dedicated dataChanged signal.

Fixes #367.

An alternative to #374.

@hebasto hebasto mentioned this pull request Jun 28, 2021
@hebasto
Copy link
Member Author

hebasto commented Jun 28, 2021

@rebroad Do you mind testing this PR?

@hebasto hebasto added Bug Something isn't working UI All about "look and feel" labels Jun 28, 2021
Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 986bf78

Tested on Arch Linux, Ubuntu, macOS 11.3, and Windows 10. The usage of dataChanged here is correct and solves the mentioned issue.

The following screenshots will compare the behavior of master and this PR. Note that the screenshots will sort by Received.

On the master screenshots you will see the ordering get out of whack because it is not dynamically sorting the peers as the data changes.

On the PR screenshots, you will see that the ordering is correct because we now allow to dynamically sort the peers as the data changes.

Ubuntu

Master PR
master pr

macOS

Master PR
master pr

Windows 10

Master PR
master pr

@hebasto hebasto added this to the 22.0 milestone Jun 29, 2021
@hebasto hebasto merged commit a62fc35 into bitcoin-core:master Jul 5, 2021
@hebasto hebasto deleted the 210628-sort branch July 5, 2021 21:03
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jul 5, 2021
…-sort Peers table

986bf78 qt: Emit dataChanged signal to dynamically re-sort Peers table (Hennadii Stepanov)

Pull request description:

  [By default](https://doc.qt.io/qt-5/qsortfilterproxymodel.html#details), the `PeerTableSortProxy`
  > dynamically re-sorts ... data whenever the original model changes.

  That is not the case on master (8cdf917) as in ecbd911 (#164) no signals are emitted to notify about model changes.

  This PR uses a dedicated [`dataChanged`](https://doc.qt.io/qt-5/qabstractitemmodel.html#dataChanged) signal.

  Fixes #367.

  An alternative to #374.

ACKs for top commit:
  jarolrod:
    ACK 986bf78

Tree-SHA512: dcb92c2f9a2c632880429e9528007db426d2ad938c64dfa1f1538c03e4b62620df52ad7daf33b582976c67b472ff76bc0dae707049f4bbbd4941232cee9ce3d4
gwillen pushed a commit to ElementsProject/elements that referenced this pull request Jun 1, 2022
@bitcoin-core bitcoin-core locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Something isn't working UI All about "look and feel"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Peer table (node window) does not remain sorted on update
2 participants