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

Introduce and Use NetworkIndicator component #251

Merged
merged 4 commits into from
Feb 9, 2023

Conversation

jarolrod
Copy link
Member

@jarolrod jarolrod commented Feb 7, 2023

Signals to the user when they are running on a network other than main.

Closes #233

Light

testnet signet regtest
Screen Shot 2023-02-08 at 6 48 47 PM Screen Shot 2023-02-08 at 6 39 00 PM Screen Shot 2023-02-08 at 6 39 21 PM

Dark

testnet signet regtest
Screen Shot 2023-02-07 at 5 24 23 PM Screen Shot 2023-02-07 at 5 24 50 PM Screen Shot 2023-02-07 at 5 25 34 PM

Windows
Intel macOS
Apple Silicon macOS
ARM64 Android

@jarolrod jarolrod mentioned this pull request Feb 7, 2023
@jarolrod jarolrod force-pushed the current-network-indicator branch from bcb3aee to dd76b6e Compare February 9, 2023 01:43
@jarolrod
Copy link
Member Author

jarolrod commented Feb 9, 2023

Updated from bcb3aee to dd76b6e, compare

  • Changes: I've been informed by @GBKS that the text should always be white

@hebasto
Copy link
Member

hebasto commented Feb 9, 2023

Rebase?

Represents the yellow-orange color used for the NetworkIndicator when
the app is running on the Signet network.
Signals to the user what network they are running under when not on the
main network.
@jarolrod jarolrod force-pushed the current-network-indicator branch from dd76b6e to 932622e Compare February 9, 2023 08:58
@jarolrod
Copy link
Member Author

jarolrod commented Feb 9, 2023

Updated from dd76b6e to 932622e, compare

changes: rebased over main

johnny9
johnny9 approved these changes Feb 9, 2023
Copy link
Contributor

@johnny9 johnny9 left a comment

Choose a reason for hiding this comment

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

minor whitespace issue.

src/qml/pages/node/NodeRunner.qml Outdated Show resolved Hide resolved
@jarolrod jarolrod force-pushed the current-network-indicator branch from 932622e to 01b3909 Compare February 9, 2023 16:31
@jarolrod
Copy link
Member Author

jarolrod commented Feb 9, 2023

Updated from 932622e to 01b3909, compare

changes: addressed indenting issue but also removed the id for the block clock component since its not being used here.

Copy link
Contributor

@johnny9 johnny9 left a comment

Choose a reason for hiding this comment

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

ACK 01b3909

Changes look good, no errors, and matches the figma

Screenshot from 2023-02-09 11-53-42
Screenshot from 2023-02-09 11-54-05

@hebasto hebasto merged commit 15df21c into bitcoin-core:main Feb 9, 2023
@GBKS
Copy link
Contributor

GBKS commented Feb 10, 2023

Sorry for the late feedback. A couple of details don't match the design. The font should be 15px, and vertical spacing should be 2 (horizontal remains 7). As is right now, the indicator looks slightly too big compared to everything else.

hebasto added a commit that referenced this pull request Feb 13, 2023
e621826 qml: fix NetworkIndicator font size and padding (jarolrod)

Pull request description:

  Addresses [this](#251 (comment)) review feedback, and gets us to fit with the [design file](https://www.figma.com/file/ek8w3n3upbluw5UL2lGhRx/Bitcoin-Core-App-Design?node-id=6010%3A67918&t=N1q0V96d7QISBeRs-4). Overlooked in #251

  ### Master

  | testnet | signet | regtest |
  | ------- | ------ | ------- |
  | <img width="752" alt="testnet-master" src="https://user-images.githubusercontent.com/23396902/218172893-9ad1b653-1a25-4fe6-9429-e422b1f799ca.png"> | <img width="752" alt="signet-master" src="https://user-images.githubusercontent.com/23396902/218172938-ac394c8b-608e-444c-89c2-804e953a7b9e.png"> | <img width="752" alt="regtest" src="https://user-images.githubusercontent.com/23396902/218172980-a23d0329-ae53-4719-8d25-f20f8d48cbac.png"> |

  ### PR

  | testnet | signet | regtest |
  | ------- | ------ | ------- |
  | <img width="752" alt="testnet-pr" src="https://user-images.githubusercontent.com/23396902/218173081-e610be32-c78c-46c9-a741-1e454bbe7a9f.png"> | <img width="752" alt="signet-pr" src="https://user-images.githubusercontent.com/23396902/218173130-483afeb1-bb25-4d53-a9f2-3e2da755ae39.png"> | <img width="752" alt="regtest-pr" src="https://user-images.githubusercontent.com/23396902/218173172-26449e22-5f32-4ad7-8289-5aa5e8d98428.png"> |

  [![Windows](https://img.shields.io/badge/OS-Windows-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/win64/insecure_win_gui.zip?branch=pull/<PR>)
  [![Intel macOS](https://img.shields.io/badge/OS-Intel%20macOS-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/macos/insecure_mac_gui.zip?branch=pull/<PR>)
  [![Apple Silicon macOS](https://img.shields.io/badge/OS-Apple%20Silicon%20macOS-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/macos_arm64/insecure_mac_arm64_gui.zip?branch=pull/<PR>)
  [![ARM64 Android](https://img.shields.io/badge/OS-Android-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/android/insecure_android_apk.zip?branch=pull/<PR>)

ACKs for top commit:
  johnny9:
    ACK e621826

Tree-SHA512: 3094cd1e9fb7f9c0e6e276a80cfa190e9ae80991d6029fdbe793485dcaea76a14e4c5a9734a840b374f04653557d7ca33dd4074020565e81fcef7735abeb080e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce Current Network Indicator
4 participants