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

qt: "Peers" tab overhaul #15136

Merged
merged 2 commits into from Jan 16, 2019
Merged

qt: "Peers" tab overhaul #15136

merged 2 commits into from Jan 16, 2019

Conversation

@hebasto
Copy link
Member

@hebasto hebasto commented Jan 9, 2019

This is an alternative to #14798.

The "Peers" tab of the "Debug" window improved to address comments #6209 (comment) (by @jonasschnelli) and #14798 (comment) (by @promag).

This allows to keep the peer selection while navigating to other places and effectively reverts e059726.

Screenshots with this PR:
screenshot from 2019-01-09 22-01-36
screenshot from 2019-01-09 22-02-11
screenshot from 2019-01-09 22-02-37

hebasto added 2 commits Jan 9, 2019
Using the QSplitter and QScrollArea classes.
Effectevely reverts e059726 commit.
@DrahtBot
Copy link
Contributor

@DrahtBot DrahtBot commented Jan 9, 2019

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #14810 (qt: Enable tabbing through labels by hebasto)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@fanquake fanquake added the GUI label Jan 9, 2019
@jonasschnelli
Copy link
Member

@jonasschnelli jonasschnelli commented Jan 10, 2019

Did a quick test and looks good (https://bitcoin.jonasschnelli.ch/build/946).

utACK 3537c83

@promag
Copy link
Member

@promag promag commented Jan 10, 2019

tACK 3537c83 on macOS 10.14.2, keeping the selection looks better to me.

Will review changes to debugwindow.ui.

</spacer>
</item>
</layout>
<widget class="QWidget" name="widget_1" native="true">

This comment has been minimized.

@promag

promag Jan 14, 2019
Member

This could be the table?

This comment has been minimized.

@hebasto

hebasto Jan 14, 2019
Author Member

QSplitter Class docs:

Note: Adding a QLayout to a QSplitter is not supported (either through setLayout() or making the QSplitter a parent of the QLayout); use addWidget() instead.

So, it cannot be a table or other kind of QLayout.

@laanwj
Copy link
Member

@laanwj laanwj commented Jan 16, 2019

utACK 3537c83

@laanwj laanwj merged commit 3537c83 into bitcoin:master Jan 16, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
laanwj added a commit that referenced this pull request Jan 16, 2019
3537c83 Do not deselect peer when switching away from tab (Hennadii Stepanov)
b0037c5 Improve Peers tab layout (Hennadii Stepanov)

Pull request description:

  This is an alternative to #14798.

  The "Peers" tab of the "Debug" window improved to address comments #6209 (comment) (by @jonasschnelli) and #14798 (comment) (by @promag).

  This allows to keep the peer selection while navigating to other places and effectively reverts e059726.

  Screenshots with this PR:
  ![screenshot from 2019-01-09 22-01-36](https://user-images.githubusercontent.com/32963518/50927352-2e6fb700-1460-11e9-9173-582348210492.png)
  ![screenshot from 2019-01-09 22-02-11](https://user-images.githubusercontent.com/32963518/50927354-329bd480-1460-11e9-9926-d0eb0f026a35.png)
  ![screenshot from 2019-01-09 22-02-37](https://user-images.githubusercontent.com/32963518/50927358-3596c500-1460-11e9-864d-c8704451f3d9.png)

Tree-SHA512: 3d086007f6d72930bc2fc3c395175adda0f1a7722de3842bc246ee4f3bfc5ebda4b9a626fb68a7ee8663a88d0842deb37c0c460ad84cc58e22f138acf8bc71ea
@hebasto hebasto deleted the hebasto:20190109-peerstab-overhaul branch Jan 16, 2019
@HashUnlimited
Copy link
Contributor

@HashUnlimited HashUnlimited commented Jan 18, 2019

On macOS this broke the visibility of the Services (scrolled all way to the right end)

screenshot 2019-01-18 at 12 20 32

@hebasto
Copy link
Member Author

@hebasto hebasto commented Jan 18, 2019

@HashUnlimited you can adjust a view by moving QSplitter bar (marked by a dot on your screenshot) and window borders:

macos high sierra_18_01_2019_14_29_35

@HashUnlimited
Copy link
Contributor

@HashUnlimited HashUnlimited commented Jan 18, 2019

ah OK... thanks. pretty confusing though that scrolling doesn't reveal all contents.

deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Nov 28, 2020
Summary:
3537c8345c788a527bb4e1d00683ca7f8ee5fb1a Do not deselect peer when switching away from tab (Hennadii Stepanov)
b0037c51909dc55e279baa81f063c169c9735105 Improve Peers tab layout (Hennadii Stepanov)

Pull request description:

  This is an alternative to #14798.

  The "Peers" tab of the "Debug" window improved to address comments bitcoin/bitcoin#6209 (comment) (by @jonasschnelli) and bitcoin/bitcoin#14798 (comment) (by @promag).

  This allows to keep the peer selection while navigating to other places and effectively reverts e059726.

  Screenshots with this PR:
  ![screenshot from 2019-01-09 22-01-36](https://user-images.githubusercontent.com/32963518/50927352-2e6fb700-1460-11e9-9173-582348210492.png)
  ![screenshot from 2019-01-09 22-02-11](https://user-images.githubusercontent.com/32963518/50927354-329bd480-1460-11e9-9926-d0eb0f026a35.png)
  ![screenshot from 2019-01-09 22-02-37](https://user-images.githubusercontent.com/32963518/50927358-3596c500-1460-11e9-864d-c8704451f3d9.png)

---

Backport of Core [[bitcoin/bitcoin#15136 | PR15136]]

Test Plan:
  ninja all
  ./qt/bitcoin-qt -testnet

check out the Peers tab for correctness

Reviewers: #bitcoin_abc, PiRK

Reviewed By: #bitcoin_abc, PiRK

Differential Revision: https://reviews.bitcoinabc.org/D8547
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants