Skip to content

Introduce ExternalLink control#141

Merged
hebasto merged 1 commit into
bitcoin-core:mainfrom
jarolrod:external-link-control
Aug 11, 2022
Merged

Introduce ExternalLink control#141
hebasto merged 1 commit into
bitcoin-core:mainfrom
jarolrod:external-link-control

Conversation

@jarolrod
Copy link
Copy Markdown
Contributor

@jarolrod jarolrod commented Aug 3, 2022

Control to handle storing and opening of external links. Can be just text or icon, or combination. To be tested with #143

Windows
Intel macOS
Apple Silicon macOS
ARM64 Android

Copy link
Copy Markdown
Contributor

@shaavan shaavan left a comment

Choose a reason for hiding this comment

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

Concept ACK

Keeping #143 in mind, porting the code related to creating an external link to separate files makes sense for the following reasons:

  1. This will allows the component to be used separately from the Settings.qml component.
  2. This will allow link-related code to be easily maintainable and also more intuitive to use compared to the current implementation.

Comment thread src/qml/controls/ExternalLink.qml Outdated
@jarolrod jarolrod force-pushed the external-link-control branch from 419bd21 to b700fc6 Compare August 4, 2022 02:23
@jarolrod
Copy link
Copy Markdown
Contributor Author

jarolrod commented Aug 4, 2022

Updated from 419bd21 to b700fc6 (pr141.01 -> pr141.02, diff)

changes:

  • make link a required property.
  • remove uncessary and redundant layout formatting declarations.
  • remove wordwrap, this was a part of the implementaion in information.qml but this is not needed at all according to the design specs, if anything a follow-up can add eliding instead. Removing word-wrap eliminates the formatting issue with long url descriptions seen in Extend settings component #143.
  • Refactor the component holding the icon from Image to Button.
    • we want the icon to be represented by the Icon type because the icon will actually appear sharper on screen, and because we can't colorize an Image without QtGraphicalEffects. If we added support now for QtGraphicalEffects it is endlessly more complex to colorize an Image than an Icon.
  • properly colorize text and icon to the design file specifications
  • fixed indentation
  • edit commit message to reflect that this is introducing a control and not a component.

@jarolrod jarolrod force-pushed the external-link-control branch from b700fc6 to 7d1d977 Compare August 5, 2022 23:28
@jarolrod
Copy link
Copy Markdown
Contributor Author

jarolrod commented Aug 5, 2022

updated from b700fc6 to 7d1d977 (pr141.02 -> pr141.03, diff)

changes: handle undefined state of description and iconSource by setting it as an empty string

Control to handle storing and opening of external links. Can be just text or icon, or combination.
@jarolrod jarolrod force-pushed the external-link-control branch from 7d1d977 to aadee41 Compare August 6, 2022 23:19
@jarolrod
Copy link
Copy Markdown
Contributor Author

jarolrod commented Aug 6, 2022

updated from 7d1d977 to aadee41 (pr141.03 -> pr141.04, diff)

Changes: make spacing in the RowLayout 0, as the design spec specifies no spacing between the link description and the icon box.

@jarolrod jarolrod changed the title Introduce ExternalLink component Introduce ExternalLink control Aug 9, 2022
Copy link
Copy Markdown
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK aadee41, tested on Ubuntu 22.04.

@hebasto hebasto merged commit 12d0f7a into bitcoin-core:main Aug 11, 2022
@jarolrod jarolrod deleted the external-link-control branch August 21, 2022 19:43
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 7, 2025
Control to handle storing and opening of external links. Can be just text or icon, or combination.

Github-Pull: bitcoin-core#141
Rebased-From: aadee41
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 8, 2025
Control to handle storing and opening of external links. Can be just text or icon, or combination.

Github-Pull: bitcoin-core#141
Rebased-From: aadee41
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 8, 2025
Control to handle storing and opening of external links. Can be just text or icon, or combination.

Github-Pull: bitcoin-core#141
Rebased-From: aadee41
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
Control to handle storing and opening of external links. Can be just text or icon, or combination.

Github-Pull: bitcoin-core#141
Rebased-From: aadee41
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
Control to handle storing and opening of external links. Can be just text or icon, or combination.

Github-Pull: bitcoin-core#141
Rebased-From: aadee41
johnny9 pushed a commit to johnny9/bitcoin-core-app that referenced this pull request Jul 4, 2025
tx-signer450 added a commit to tx-signer450/gui-qml that referenced this pull request Oct 20, 2025
5cbad1336af11a832d0e48e6245ad2cccaf1b841 qml: introduce ExternalLink control (jarolrod)

Pull request description:

  Control to handle storing and opening of external links. Can be just text or icon, or combination. To be tested with #143

  [![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/141)
  [![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/141)
  [![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/141)
  [![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/141)

ACKs for top commit:
  hebasto:
    ACK 5cbad1336af11a832d0e48e6245ad2cccaf1b841, tested on Ubuntu 22.04.

Tree-SHA512: 01a1c456607742ac0ec4fe4a575631a9ab6cc6dc263c773ad2673b69534ce0463fcd8915b6fd9f11c32c8e9315fc204e55fec545dfcaddeb6bd337f2a7708ced
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.

3 participants