Skip to content

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

Merged
hebasto merged 1 commit intobitcoin-core:masterfrom
hebasto:210323-peers
Jun 5, 2021
Merged

Save/restore column sizes of the tables in the Peers tab#256
hebasto merged 1 commit intobitcoin-core:masterfrom
hebasto:210323-peers

Conversation

@hebasto
Copy link
Member

@hebasto hebasto commented Mar 23, 2021

No description provided.

@hebasto hebasto mentioned this pull request Mar 26, 2021
Merged
jarolrod
jarolrod reviewed Mar 26, 2021
View reviewed changes
Copy link
Contributor

@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

jonatack
jonatack approved these changes Mar 26, 2021
View reviewed changes
Copy link
Member

@jonatack jonatack left a comment •
edited
Loading

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 hebasto closed this Mar 28, 2021
@hebasto hebasto reopened this Mar 28, 2021
@hebasto hebasto closed this Mar 28, 2021
@hebasto hebasto reopened this Mar 28, 2021
@hebasto hebasto mentioned this pull request Mar 29, 2021
Closed
@hebasto
Copy link
Member Author

hebasto commented Apr 20, 2021

@promag

Mind looking into this PR?

This was referenced Apr 23, 2021
Closed
Closed
promag
promag reviewed Apr 23, 2021
View reviewed changes
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.

@RandyMcMillan RandyMcMillan mentioned this pull request Apr 23, 2021
Closed
@hebasto hebasto mentioned this pull request Apr 25, 2021
Open
@hebasto
Copy link
Member Author

hebasto commented Apr 29, 2021

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

@jarolrod jarolrod mentioned this pull request May 3, 2021
Merged
promag
promag reviewed May 5, 2021
View reviewed changes
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.

jarolrod
jarolrod reviewed May 13, 2021
View reviewed changes
Copy link
Contributor

@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

@RandyMcMillan RandyMcMillan mentioned this pull request May 17, 2021
Closed
hebasto added a commit that referenced this pull request May 26, 2021
@hebasto
Merge #311: Peers Window rename 'Peer id' to 'Peer'
5c041cb 
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.

jonatack
jonatack reviewed Jun 5, 2021
View reviewed changes
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.

jarolrod
jarolrod reviewed Jun 5, 2021
View reviewed changes
Copy link
Contributor

@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
@hebasto hebasto mentioned this pull request Jun 5, 2021
Closed
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jun 6, 2021
@hebasto
Merge bitcoin-core/gui#256: Save/restore column sizes of the tables i…
9750498 
…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
@jarolrod jarolrod mentioned this pull request Jul 6, 2021
Merged
gwillen pushed a commit to ElementsProject/elements that referenced this pull request Jun 1, 2022
@apoelstra
Merge 21d87bb into merged_master (Bitcoin PR bitcoin-core/gui#256)
9e27d6c
@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.

Reviewers

3 more reviewers

@jonatack jonatack jonatack approved these changes

@promag promag promag left review comments

@jarolrod jarolrod jarolrod left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Feature

Projects

None yet

Milestone

22.0

Development

Successfully merging this pull request may close these issues.

5 participants

@hebasto @jonatack @promag @jarolrod @DrahtBot