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: Add vertical spacer to peer detail widget #16090

Merged
merged 1 commit into from Jun 3, 2019

Conversation

@JosuGZ
Copy link
Contributor

commented May 25, 2019

Before:

image

After:

image

@DrahtBot DrahtBot added the GUI label May 25, 2019
@practicalswift

This comment has been minimized.

Copy link
Member

commented May 26, 2019

Concept ACK: looks much better!

@JosuGZ, welcome as a contributor! Hope to see more contributions from you! :-)

@JosuGZ

This comment has been minimized.

Copy link
Contributor Author

commented May 26, 2019

Concept ACK: looks much better!

@JosuGZ, welcome as a contributor! Hope to see more contributions from you! :-)

Thanks! I have no clue about why that rpc_psbt.py test fails though.

@practicalswift

This comment has been minimized.

Copy link
Member

commented May 26, 2019

@joshrabinowitz That is just a random unrelated failure :-)

Copy link
Contributor

left a comment

ACK 36b0a2f (tested with Qt 5.11.3 under Linux/Xfce4)

Copy link
Member

left a comment

Tested ACK 36b0a2f on macos 10.14.3. Resizing the window works as expected.

<property name="orientation">
<enum>Qt::Vertical</enum>
</property>
<property name="sizeHint" stdset="0">

This comment has been minimized.

Copy link
@promag

promag May 27, 2019

Member

Property could be removed.

This comment has been minimized.

Copy link
@JosuGZ

JosuGZ May 28, 2019

Author Contributor

It seems indeed unneeded, I think it is how Qt creator creates spacers though. Change pushed.

This comment has been minimized.

Copy link
@JosuGZ

JosuGZ May 28, 2019

Author Contributor

Hmm, I'm noticing other spacers have the property set, like the one in the information tab on the same file.

@fanquake

This comment has been minimized.

Copy link
Member

commented May 28, 2019

utACK 36b0a2f

This change doesn't seem to make any difference on macOS (10.14.x), as the before behavior isn't present.

@JosuGZ What OS are you seeing this issue on?

@promag

This comment has been minimized.

Copy link
Member

commented May 28, 2019

@fanquake I'm on a macos and it also stretches the details (macos 10.14.3, Qt 5.12.2).

@kristapsk

This comment has been minimized.

Copy link
Contributor

commented May 28, 2019

@fanquake , I have the "before" behaviour also on Linux/Xfce4, Qt 5.11.3.

@JosuGZ

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2019

@fanquake

@JosuGZ What OS are you seeing this issue on?

Linux Mint 19.1 Tessa

It usually also happens when I do this kind of layouts on Windows.

@fanquake

This comment has been minimized.

Copy link
Member

commented May 28, 2019

@promag @kristapsk @JosuGZ Thanks for clarifying.

@hebasto

This comment has been minimized.

Copy link
Member

commented May 28, 2019

tACK 36b0a2f on Linux Mint 19.1, Qt 5.9.5
Could you squash your commits?

The different behavior (some can see 'before', others not) can be caused by QGridLayout::setRowStretch:

If the stretch factor is 0 and no other row in this table can grow at all, the row may still grow.

@JosuGZ JosuGZ force-pushed the JosuGZ:gui-improvements branch from b292f5b to 36b0a2f May 28, 2019
@JosuGZ

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2019

@hebasto I have left the property for coherency with the rest of the file and with Qt Creator.

@fanquake

This comment has been minimized.

Copy link
Member

commented Jun 3, 2019

re-utACK 36b0a2f

Could have used a commit message like gui: add vertical spacer to peers window.

@JosuGZ

This comment has been minimized.

Copy link
Contributor Author

commented Jun 3, 2019

True. Shouldn't appear in the the merge commit anyway?

@laanwj laanwj merged commit 36b0a2f into bitcoin:master Jun 3, 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 Jun 3, 2019
36b0a2f Add vertical spacer (Josu Goñi)

Pull request description:

  Before:

  ![image](https://user-images.githubusercontent.com/25986871/58375408-a8f22c80-7f52-11e9-96ca-14f2186e6fa7.png)

  After:

  ![image](https://user-images.githubusercontent.com/25986871/58375420-fa022080-7f52-11e9-8add-eafe98068e8d.png)

ACKs for commit 36b0a2:
  fanquake:
    utACK 36b0a2f
  hebasto:
    tACK 36b0a2f on Linux Mint 19.1, Qt 5.9.5
  fanquake:
    re-utACK 36b0a2f
  kristapsk:
    ACK 36b0a2f (tested with Qt 5.11.3 under Linux/Xfce4)
  promag:
    Tested ACK 36b0a2f on macos 10.14.3. Resizing the window works as expected.

Tree-SHA512: 26ec9700aa9116ec2c604f8ec7b825b30c83c1d497c21f2191d3585868db4a2e3921de607dea9f7cd9a1ea49361215d738e2aba1936566d85757d87112d73088
LitecoinZ added a commit to litecoinz-core/wip that referenced this pull request Jun 5, 2019
36b0a2f Add vertical spacer (Josu Goñi)

Pull request description:

  Before:

  ![image](https://user-images.githubusercontent.com/25986871/58375408-a8f22c80-7f52-11e9-96ca-14f2186e6fa7.png)

  After:

  ![image](https://user-images.githubusercontent.com/25986871/58375420-fa022080-7f52-11e9-8add-eafe98068e8d.png)

ACKs for commit 36b0a2:
  fanquake:
    utACK bitcoin@36b0a2f
  hebasto:
    tACK 36b0a2f on Linux Mint 19.1, Qt 5.9.5
  fanquake:
    re-utACK bitcoin@36b0a2f
  kristapsk:
    ACK 36b0a2f (tested with Qt 5.11.3 under Linux/Xfce4)
  promag:
    Tested ACK bitcoin@36b0a2f on macos 10.14.3. Resizing the window works as expected.

Tree-SHA512: 26ec9700aa9116ec2c604f8ec7b825b30c83c1d497c21f2191d3585868db4a2e3921de607dea9f7cd9a1ea49361215d738e2aba1936566d85757d87112d73088
sidhujag added a commit to syscoin/syscoin that referenced this pull request Jun 6, 2019
36b0a2f Add vertical spacer (Josu Goñi)

Pull request description:

  Before:

  ![image](https://user-images.githubusercontent.com/25986871/58375408-a8f22c80-7f52-11e9-96ca-14f2186e6fa7.png)

  After:

  ![image](https://user-images.githubusercontent.com/25986871/58375420-fa022080-7f52-11e9-8add-eafe98068e8d.png)

ACKs for commit 36b0a2:
  fanquake:
    utACK bitcoin@36b0a2f
  hebasto:
    tACK 36b0a2f on Linux Mint 19.1, Qt 5.9.5
  fanquake:
    re-utACK bitcoin@36b0a2f
  kristapsk:
    ACK 36b0a2f (tested with Qt 5.11.3 under Linux/Xfce4)
  promag:
    Tested ACK bitcoin@36b0a2f on macos 10.14.3. Resizing the window works as expected.

Tree-SHA512: 26ec9700aa9116ec2c604f8ec7b825b30c83c1d497c21f2191d3585868db4a2e3921de607dea9f7cd9a1ea49361215d738e2aba1936566d85757d87112d73088
luke-jr added a commit to luke-jr/bitcoin that referenced this pull request Aug 23, 2019
Github-Pull: bitcoin#16090
Rebased-From: 36b0a2f
@luke-jr luke-jr referenced this pull request Aug 23, 2019
fanquake added a commit to fanquake/bitcoin that referenced this pull request Aug 24, 2019
Github-Pull: bitcoin#16090
Rebased-From: 36b0a2f
luke-jr added a commit to bitcoinknots/bitcoin that referenced this pull request Sep 3, 2019
Github-Pull: bitcoin#16090
Rebased-From: 36b0a2f
fanquake added a commit to fanquake/bitcoin that referenced this pull request Sep 23, 2019
Github-Pull: bitcoin#16090
Rebased-From: 36b0a2f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.