Skip to content

Add focus outline border to focusable components#264

Merged
hebasto merged 3 commits into
bitcoin-core:mainfrom
jarolrod:focus-outline
Feb 28, 2023
Merged

Add focus outline border to focusable components#264
hebasto merged 3 commits into
bitcoin-core:mainfrom
jarolrod:focus-outline

Conversation

@jarolrod
Copy link
Copy Markdown
Contributor

@jarolrod jarolrod commented Feb 14, 2023

Replaces #214

Adds the focus outline to the (intended) focusable components:

a b c d e f g h i j k
Screen Shot 2023-02-13 at 8 48 31 PM Screen Shot 2023-02-13 at 8 48 40 PM Screen Shot 2023-02-13 at 8 48 45 PM Screen Shot 2023-02-13 at 8 48 53 PM Screen Shot 2023-02-13 at 8 48 55 PM Screen Shot 2023-02-13 at 8 48 58 PM Screen Shot 2023-02-13 at 8 49 01 PM Screen Shot 2023-02-13 at 8 49 19 PM Screen Shot 2023-02-13 at 8 49 21 PM Screen Shot 2023-02-13 at 8 49 26 PM Screen Shot 2023-02-13 at 8 49 31 PM

#219 is still something we have to fix in order for this to work properly

Additionally, this insets the navbuttons 4px from the borders of the GUI to allow for the focus outline to show properly. I believe that GBKS intended the buttons to be inset a bit from the borders

a b
Screen Shot 2023-02-13 at 8 36 33 PM Screen Shot 2023-02-13 at 8 38 57 PM

Windows
Intel macOS
Apple Silicon macOS
ARM64 Android

@GBKS
Copy link
Copy Markdown
Contributor

GBKS commented Feb 14, 2023

@jarolrod looks awesome. Yes, the nav buttons are inset from the edges a bit. Looks better, and allows for the focus state to be visible.

@jarolrod
Copy link
Copy Markdown
Contributor Author

Updated from 8e6365b to a14c3a8, compare

changes: rebased over main

@hebasto
Copy link
Copy Markdown
Member

hebasto commented Feb 14, 2023

Once more rebase?

@jarolrod
Copy link
Copy Markdown
Contributor Author

updated from a14c3a8 to 48635c1, compare

changes: rebased over main

@jarolrod jarolrod mentioned this pull request Feb 15, 2023
Copy link
Copy Markdown
Collaborator

@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.

BlockClock has a bug that causes the focus outline to always be on.

Comment thread src/qml/components/BlockClock.qml Outdated
@jarolrod
Copy link
Copy Markdown
Contributor Author

Updated from 48635c1 to 350bf21, compare

changes: addressed review comment

Copy link
Copy Markdown
Collaborator

@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 350bf21

@jarolrod
Copy link
Copy Markdown
Contributor Author

Updated from 350bf21 to bae7abe, compare

Changes: changed approach so that

  • The Outline Rectangle is a reusable component called FocusBorder
  • This FocusBorder encapsulates property definitions as well as behavior animations to reduce code
  • Handle background states on tab focus for relevant controls

@jarolrod
Copy link
Copy Markdown
Contributor Author

jarolrod commented Feb 17, 2023

Updated from bae7abe to 6d43e20, compare

changes: fixed no newline at EOF

Comment thread src/qml/controls/TextButton.qml Outdated
@jarolrod
Copy link
Copy Markdown
Contributor Author

Updated from 6d43e20 to ab0ea57, compare

Changes:

  • dropped changes on checking the reasons behind the visual focus change
  • No longer change backgrounds of certain components when given focus per feedback on slack
  • squashed commits related to adding the usage of FocusBorder into one commit as the changes are now simple enough.

Copy link
Copy Markdown
Collaborator

@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 ab0ea57

@hebasto hebasto merged commit 94f1fe5 into bitcoin-core:main Feb 28, 2023
hebasto added a commit that referenced this pull request May 28, 2023
72fe12f qml: add missing topMargin to Navigation Bar middle detail (jarolrod)

Pull request description:

  This was missed in #264

  | master | pr |
  | ------ | -- |
  | <img width="642" alt="Screen Shot 2023-05-26 at 4 03 33 PM" src="https://github.com/bitcoin-core/gui-qml/assets/23396902/fc1523fd-0742-4beb-abb4-850f72a80fe3"> | <img width="642" alt="Screen Shot 2023-05-26 at 3 57 54 PM" src="https://github.com/bitcoin-core/gui-qml/assets/23396902/d643d02e-4410-4267-9521-5405039b0f53"> |

  [![Windows](https://img.shields.io/badge/OS-Windows-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/win64/unsecure_win_gui.zip?branch=pull/334)
  [![Intel macOS](https://img.shields.io/badge/OS-Intel%20macOS-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/macos/unsecure_mac_gui.zip?branch=pull/334)
  [![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/unsecure_mac_arm64_gui.zip?branch=pull/334)
  [![ARM64 Android](https://img.shields.io/badge/OS-Android-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/android/unsecure_android_apk.zip?branch=pull/334)
  [![ARM32 Android](https://img.shields.io/badge/OS-Android%2032bit-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/android32/unsecure_android_32bit_apk.zip?branch=pull/334)

ACKs for top commit:
  johnny9:
    ACK 72fe12f

Tree-SHA512: 9d94d0e031160b6dc33cf4f27d79880dfda6482527146ea129ed98460d91b2177b87404d2ac59e4ac728531444f244abd3e38b049a8ab3106db45bf8f7fbebdd
Comment on lines +32 to +34
Behavior on visible {
NumberAnimation { duration: 150 }
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is the idea? should it be opacity animation?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, a simple opacity animation is fine.

johnny9 pushed a commit to johnny9/bitcoin-core-app that referenced this pull request Jul 4, 2025
… components

25c7de6 qml: add FocusBorder to Focus-able components (jarolrod)
f0c5bf0 qml: Introduce FocusBorder component (Jarol Rodriguez)
e718e59 qml: inset NavButtons 4px from borders to give space for focus outline (jarolrod)

Pull request description:

  Replaces bitcoin-core/gui-qml#214

  Adds the focus outline to the (intended) focusable components:

  | a | b | c | d | e | f | g | h | i | j | k |
  | - | - | - | - | - | - | - | - | - | - | - |
  | <img width="752" alt="Screen Shot 2023-02-13 at 8 48 31 PM" src="https://user-images.githubusercontent.com/23396902/218634844-31045e76-5673-4d74-8606-8c1b27a3cf5e.png"> |  <img width="752" alt="Screen Shot 2023-02-13 at 8 48 40 PM" src="https://user-images.githubusercontent.com/23396902/218634877-f935fbd2-289c-4967-a31e-953daeda08d2.png"> |  <img width="752" alt="Screen Shot 2023-02-13 at 8 48 45 PM" src="https://user-images.githubusercontent.com/23396902/218634925-d42f984e-a58d-43bb-88b3-49de6a273ff0.png"> |  <img width="752" alt="Screen Shot 2023-02-13 at 8 48 53 PM" src="https://user-images.githubusercontent.com/23396902/218634957-7b0d458b-80c2-4e02-938f-20324e2235fa.png"> |  <img width="752" alt="Screen Shot 2023-02-13 at 8 48 55 PM" src="https://user-images.githubusercontent.com/23396902/218634993-ae906fc9-0410-4551-a3a8-890bacea76fc.png"> | <img width="752" alt="Screen Shot 2023-02-13 at 8 48 58 PM" src="https://user-images.githubusercontent.com/23396902/218635019-040cd823-e9f8-4e86-b35b-8f4248b9d6a8.png"> | <img width="752" alt="Screen Shot 2023-02-13 at 8 49 01 PM" src="https://user-images.githubusercontent.com/23396902/218635041-820f0e49-9abc-4b61-8a7b-ec18a927d12f.png"> |  <img width="752" alt="Screen Shot 2023-02-13 at 8 49 19 PM" src="https://user-images.githubusercontent.com/23396902/218635060-3f94b33f-2be2-443b-a681-09ee14e9c467.png"> |  <img width="752" alt="Screen Shot 2023-02-13 at 8 49 21 PM" src="https://user-images.githubusercontent.com/23396902/218635100-1a1aec9b-38c6-4d62-9265-3f9700b07d4e.png"> |  <img width="752" alt="Screen Shot 2023-02-13 at 8 49 26 PM" src="https://user-images.githubusercontent.com/23396902/218635123-ef687083-aee4-4db0-9c1d-7dd8ebfdd75c.png"> | <img width="752" alt="Screen Shot 2023-02-13 at 8 49 31 PM" src="https://user-images.githubusercontent.com/23396902/218635147-f00d6480-337f-4947-88a6-920c9f463d12.png"> |

  bitcoin-core/gui-qml#219 is still something we have to fix in order for this to work properly

  Additionally, this insets the navbuttons 4px from the borders of the GUI to allow for the focus outline to show properly. I believe that **GBKS** intended the buttons to be inset a bit from the borders

  | a | b |
  | - | - |
  | <img width="752" alt="Screen Shot 2023-02-13 at 8 36 33 PM" src="https://user-images.githubusercontent.com/23396902/218636059-13ecf932-5e41-4bb7-94bb-264523283438.png"> |  <img width="752" alt="Screen Shot 2023-02-13 at 8 38 57 PM" src="https://user-images.githubusercontent.com/23396902/218636186-499c5549-b2cc-4325-aade-022be3f47e85.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/264)
  [![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/264)
  [![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/264)
  [![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/264)

ACKs for top commit:
  johnny9:
    ACK 25c7de6

Tree-SHA512: f26ca941d0d3a8026846f3a86a7867fb437aeb3b3aca2af3054ce08c57f46f16c8536691b49a9d08dba8f14009d771f9dbddfe9e61d886ee2cc59ddcdb25db0a
johnny9 pushed a commit to johnny9/bitcoin-core-app that referenced this pull request Jul 4, 2025
…ar middle detail

eca46cb qml: add missing topMargin to Navigation Bar middle detail (jarolrod)

Pull request description:

  This was missed in bitcoin-core/gui-qml#264

  | master | pr |
  | ------ | -- |
  | <img width="642" alt="Screen Shot 2023-05-26 at 4 03 33 PM" src="https://github.com/bitcoin-core/gui-qml/assets/23396902/fc1523fd-0742-4beb-abb4-850f72a80fe3"> | <img width="642" alt="Screen Shot 2023-05-26 at 3 57 54 PM" src="https://github.com/bitcoin-core/gui-qml/assets/23396902/d643d02e-4410-4267-9521-5405039b0f53"> |

  [![Windows](https://img.shields.io/badge/OS-Windows-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/win64/unsecure_win_gui.zip?branch=pull/334)
  [![Intel macOS](https://img.shields.io/badge/OS-Intel%20macOS-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/macos/unsecure_mac_gui.zip?branch=pull/334)
  [![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/unsecure_mac_arm64_gui.zip?branch=pull/334)
  [![ARM64 Android](https://img.shields.io/badge/OS-Android-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/android/unsecure_android_apk.zip?branch=pull/334)
  [![ARM32 Android](https://img.shields.io/badge/OS-Android%2032bit-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/android32/unsecure_android_32bit_apk.zip?branch=pull/334)

ACKs for top commit:
  johnny9:
    ACK eca46cb

Tree-SHA512: 9d94d0e031160b6dc33cf4f27d79880dfda6482527146ea129ed98460d91b2177b87404d2ac59e4ac728531444f244abd3e38b049a8ab3106db45bf8f7fbebdd
tx-signer450 added a commit to tx-signer450/gui-qml that referenced this pull request Oct 20, 2025
… components

25c7de695c9d0a7de105bf97f7ebb3a5968c131d qml: add FocusBorder to Focus-able components (jarolrod)
f0c5bf01b496095ac110120b6b0110fd1d28fee5 qml: Introduce FocusBorder component (Jarol Rodriguez)
e718e59dfb9726fe62b89ccf5a452daddb0f5432 qml: inset NavButtons 4px from borders to give space for focus outline (jarolrod)

Pull request description:

  Replaces bitcoin-core/gui-qml#214

  Adds the focus outline to the (intended) focusable components:

  | a | b | c | d | e | f | g | h | i | j | k |
  | - | - | - | - | - | - | - | - | - | - | - |
  | <img width="752" alt="Screen Shot 2023-02-13 at 8 48 31 PM" src="https://user-images.githubusercontent.com/23396902/218634844-31045e76-5673-4d74-8606-8c1b27a3cf5e.png"> |  <img width="752" alt="Screen Shot 2023-02-13 at 8 48 40 PM" src="https://user-images.githubusercontent.com/23396902/218634877-f935fbd2-289c-4967-a31e-953daeda08d2.png"> |  <img width="752" alt="Screen Shot 2023-02-13 at 8 48 45 PM" src="https://user-images.githubusercontent.com/23396902/218634925-d42f984e-a58d-43bb-88b3-49de6a273ff0.png"> |  <img width="752" alt="Screen Shot 2023-02-13 at 8 48 53 PM" src="https://user-images.githubusercontent.com/23396902/218634957-7b0d458b-80c2-4e02-938f-20324e2235fa.png"> |  <img width="752" alt="Screen Shot 2023-02-13 at 8 48 55 PM" src="https://user-images.githubusercontent.com/23396902/218634993-ae906fc9-0410-4551-a3a8-890bacea76fc.png"> | <img width="752" alt="Screen Shot 2023-02-13 at 8 48 58 PM" src="https://user-images.githubusercontent.com/23396902/218635019-040cd823-e9f8-4e86-b35b-8f4248b9d6a8.png"> | <img width="752" alt="Screen Shot 2023-02-13 at 8 49 01 PM" src="https://user-images.githubusercontent.com/23396902/218635041-820f0e49-9abc-4b61-8a7b-ec18a927d12f.png"> |  <img width="752" alt="Screen Shot 2023-02-13 at 8 49 19 PM" src="https://user-images.githubusercontent.com/23396902/218635060-3f94b33f-2be2-443b-a681-09ee14e9c467.png"> |  <img width="752" alt="Screen Shot 2023-02-13 at 8 49 21 PM" src="https://user-images.githubusercontent.com/23396902/218635100-1a1aec9b-38c6-4d62-9265-3f9700b07d4e.png"> |  <img width="752" alt="Screen Shot 2023-02-13 at 8 49 26 PM" src="https://user-images.githubusercontent.com/23396902/218635123-ef687083-aee4-4db0-9c1d-7dd8ebfdd75c.png"> | <img width="752" alt="Screen Shot 2023-02-13 at 8 49 31 PM" src="https://user-images.githubusercontent.com/23396902/218635147-f00d6480-337f-4947-88a6-920c9f463d12.png"> |

  bitcoin-core/gui-qml#219 is still something we have to fix in order for this to work properly

  Additionally, this insets the navbuttons 4px from the borders of the GUI to allow for the focus outline to show properly. I believe that **GBKS** intended the buttons to be inset a bit from the borders

  | a | b |
  | - | - |
  | <img width="752" alt="Screen Shot 2023-02-13 at 8 36 33 PM" src="https://user-images.githubusercontent.com/23396902/218636059-13ecf932-5e41-4bb7-94bb-264523283438.png"> |  <img width="752" alt="Screen Shot 2023-02-13 at 8 38 57 PM" src="https://user-images.githubusercontent.com/23396902/218636186-499c5549-b2cc-4325-aade-022be3f47e85.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/264)
  [![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/264)
  [![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/264)
  [![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/264)

ACKs for top commit:
  johnny9:
    ACK 25c7de695c9d0a7de105bf97f7ebb3a5968c131d

Tree-SHA512: f26ca941d0d3a8026846f3a86a7867fb437aeb3b3aca2af3054ce08c57f46f16c8536691b49a9d08dba8f14009d771f9dbddfe9e61d886ee2cc59ddcdb25db0a
tx-signer450 added a commit to tx-signer450/gui-qml that referenced this pull request Oct 20, 2025
…ar middle detail

eca46cbea63ab68ae7c8bcd962fe9e10d738e5fd qml: add missing topMargin to Navigation Bar middle detail (jarolrod)

Pull request description:

  This was missed in bitcoin-core/gui-qml#264

  | master | pr |
  | ------ | -- |
  | <img width="642" alt="Screen Shot 2023-05-26 at 4 03 33 PM" src="https://github.com/bitcoin-core/gui-qml/assets/23396902/fc1523fd-0742-4beb-abb4-850f72a80fe3"> | <img width="642" alt="Screen Shot 2023-05-26 at 3 57 54 PM" src="https://github.com/bitcoin-core/gui-qml/assets/23396902/d643d02e-4410-4267-9521-5405039b0f53"> |

  [![Windows](https://img.shields.io/badge/OS-Windows-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/win64/unsecure_win_gui.zip?branch=pull/334)
  [![Intel macOS](https://img.shields.io/badge/OS-Intel%20macOS-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/macos/unsecure_mac_gui.zip?branch=pull/334)
  [![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/unsecure_mac_arm64_gui.zip?branch=pull/334)
  [![ARM64 Android](https://img.shields.io/badge/OS-Android-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/android/unsecure_android_apk.zip?branch=pull/334)
  [![ARM32 Android](https://img.shields.io/badge/OS-Android%2032bit-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/android32/unsecure_android_32bit_apk.zip?branch=pull/334)

ACKs for top commit:
  johnny9:
    ACK eca46cbea63ab68ae7c8bcd962fe9e10d738e5fd

Tree-SHA512: 9d94d0e031160b6dc33cf4f27d79880dfda6482527146ea129ed98460d91b2177b87404d2ac59e4ac728531444f244abd3e38b049a8ab3106db45bf8f7fbebdd
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.

5 participants