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

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

Merged
merged 1 commit into from
Jun 5, 2021

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Mar 23, 2021

No description provided.

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 dda2f65, 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

Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

Very practical improvement.

ACK dda2f65 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.

@hebasto
Copy link
Member Author

hebasto commented Apr 20, 2021

@promag

Mind looking into this PR?

Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

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

Tested ACK dda2f65.

src/qt/rpcconsole.cpp Show resolved Hide resolved
src/qt/rpcconsole.cpp Outdated Show resolved Hide resolved
@hebasto
Copy link
Member Author

hebasto commented Apr 29, 2021

Updated dda2f65 -> 002d0e0 (pr256.01 -> pr256.02, diff):

Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

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

Code review ACK 002d0e0.

@hebasto hebasto added this to the 22.0 milestone May 8, 2021
@hebasto
Copy link
Member Author

hebasto commented May 8, 2021

@jonatack @jarolrod

Do you mind re-reviewing this PR?

@hebasto
Copy link
Member Author

hebasto commented May 10, 2021

Rebased 002d0e0 -> 0337044 (pr256.02 -> pr256.03) due to the conflict with #194.

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 0337044

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

hebasto added a commit that referenced this pull request May 26, 2021
657b33e qt: add translator comments for peers table columns (Jarol Rodriguez)
73a91c6 gui: rename "Peer Id" to "Peer" in tab column and details area (Jon Atack)

Pull request description:

  Picking up #290

  **Original PR Description:**
  - renames the peers tab column header from `Peer Id` to `Peer` to allow resizing the column more tightly (this will be particularly useful after #256) and does the same for the peer details area.

  While here, we also add Qt translator comments for the Peer Table columns.

  | Master        | PR               |
  | ----------- | ----------- |
  | ![Screen Shot 2021-05-03 at 1 23 05 AM](https://user-images.githubusercontent.com/23396902/116843818-20a14b00-abaf-11eb-913e-ddff11cda5cd.png) | ![Screen Shot 2021-05-05 at 4 08 45 AM](https://user-images.githubusercontent.com/23396902/117112825-a2cc7380-ad57-11eb-939b-1aceb4214ad1.png) |

ACKs for top commit:
  jonatack:
    utACK 657b33e
  hebasto:
    re-ACK 657b33e

Tree-SHA512: f50116f7ca8719cadf1f95f45e3594b3b92bde9c43eb954f3e963ed10629dd9406efdb5e96aa1f750a926e24a96321d824ed3780bd9cd748774e0b85fd0c9535
@hebasto
Copy link
Member Author

hebasto commented May 29, 2021

@jonatack @promag

Hoping on confirming your ACKs after the recent rebasing :)

@hebasto
Copy link
Member Author

hebasto commented Jun 5, 2021

Rebased 0337044 -> fb1b1e0 (pr256.03 -> pr256.04) due to the conflict with #123.

Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

This is a real UX improvement, thanks!

ACK fb1b1e0 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 38ab7d0. I don't recall seeing it before but am not sure.

@hebasto
Copy link
Member Author

hebasto commented Jun 5, 2021

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

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 fb1b1e0

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

@hebasto hebasto merged commit 21d87bb into bitcoin-core:master Jun 5, 2021
@hebasto hebasto deleted the 210323-peers branch June 5, 2021 20:03
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jun 6, 2021
…n the Peers tab

fb1b1e0 qt: Save/restore column sizes of the tables in the Peers tab (Hennadii Stepanov)

Pull request description:

ACKs for top commit:
  jonatack:
    ACK fb1b1e0 code review, debug-built and tested
  jarolrod:
    ACK fb1b1e0

Tree-SHA512: f93495ecd13e4202aba61b407fffbeec855f5b0c1cc027197c78edddd7d11c87ebdb0fcb1daac242f0407323b31f4e7e0313bd76113a5241e4c868a8829af20a
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants