Skip to content

Add OptionButton control #67

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Nov 3, 2021
Merged

Add OptionButton control #67

merged 7 commits into from
Nov 3, 2021

Conversation

promag
Copy link
Contributor

@promag promag commented Oct 22, 2021 •
edited
Loading

image

Windows
macOS

@hebasto
Copy link
Member

hebasto commented Oct 22, 2021

Rebase on top of the merged #49?

@hebasto
Copy link
Member

hebasto commented Oct 22, 2021

Concept ACK. Looks great!

@hebasto hebasto added the UX Designers' opinions are required label Oct 22, 2021
@promag promag force-pushed the 2021-10-option-button branch 2 times, most recently from a6c389f to 99d4f66 Compare October 22, 2021 17:19
@promag
Copy link
Contributor Author

promag commented Oct 22, 2021 •
edited
Loading

Rebased and split to a new component (ConnectionOptions) where OptionButton is used.

@hebasto
Copy link
Member

hebasto commented Oct 22, 2021

Tested 99d4f66:

qrc:/qml/pages/stub.qml:22:5: QML ColumnLayout: Possible anchor loop detected on centerIn.

@promag
Copy link
Contributor Author

promag commented Oct 22, 2021

@hebasto thanks, didn't notice it, will fix.

@promag promag force-pushed the 2021-10-option-button branch from 99d4f66 to 653d7cd Compare October 22, 2021 21:47
@promag
Copy link
Contributor Author

promag commented Oct 22, 2021

@hebasto see 2390239 description.

@promag
Copy link
Contributor Author

promag commented Oct 22, 2021

0b5fb6b is included so that you can test by pressing TAB to switch between options and SPACE to select it (we don't have focus feedback yet).

@hebasto
Copy link
Member

hebasto commented Oct 27, 2021 •
edited
Loading

0b5fb6b is included so that you can test by pressing TAB to switch between options and SPACE to select it (we don't have focus feedback yet).

Does not work for me on Linux. Only mouse clicks work.

@hebasto
Copy link
Member

hebasto commented Oct 27, 2021

2acd4a7 "qml: Drop import qualifier BitcoinCoreComponents"

Wouldn't it be useful to keep it as is in case if we use import "../components" and import "../controls" in the same QML file?

@promag
Copy link
Contributor Author

promag commented Oct 27, 2021

0b5fb6b is included so that you can test by pressing TAB to switch between options and SPACE to select it (we don't have focus feedback yet).

Does not work for me on Linux. Only mouse clicks work.

Built on ubuntu with depends and works by pressing TAB and SPACE. I'll include focus feedback since @GBKS already has a suggestion for that.

@promag
Copy link
Contributor Author

promag commented Oct 27, 2021

2acd4a7 "qml: Drop import qualifier BitcoinCoreComponents"

Wouldn't it be useful to keep it as is in case if we use import "../components" and import "../controls" in the same QML file?

That should be fine, I don't expect name collisions - at least I'd like to avoid ambiguities.

@promag promag force-pushed the 2021-10-option-button branch from 653d7cd to 4c4aa30 Compare October 27, 2021 22:25
@promag
Copy link
Contributor Author

promag commented Oct 27, 2021

Now with hover and focus feedback.

Rebased on top of #73 to have the macOS artifact.

@promag promag force-pushed the 2021-10-option-button branch 2 times, most recently from e143115 to 6af285e Compare October 28, 2021 14:47
@promag
Copy link
Contributor Author

promag commented Oct 28, 2021

Rebased, made adjustments suggested by @GBKS, made ConnectionOptions extend Pane to better handle keyboard focus, set 1st option as the default control to have keyboard focus.

@promag promag force-pushed the 2021-10-option-button branch from 6af285e to b70ee53 Compare October 29, 2021 08:23
@promag
Copy link
Contributor Author

promag commented Oct 29, 2021

Rebased on #74.

@promag promag force-pushed the 2021-10-option-button branch 2 times, most recently from b70ee53 to 7cde50b Compare October 29, 2021 13:45
hebasto
hebasto reviewed Oct 30, 2021
View reviewed changes
Copy link
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.

Tested 7cde50b on Linux Mint 20.2 (Qt 5.12.8).

Tab (focus moving) and Space (focus selecting) work as expected.

What should be a role of the Return key in focus treatment?

Also:

$ src/qt/bitcoin-qt -signet
qrc:/qml/pages/stub.qml:34:9: QML Pane: Binding loop detected for property "implicitHeight"
qrc:/qml/pages/stub.qml:34:9: QML Pane: Binding loop detected for property "implicitHeight"

@promag
Copy link
Contributor Author

promag commented Oct 30, 2021

Thanks @hebasto, will address both comments.

promag
promag commented Oct 30, 2021
View reviewed changes
promag added 2 commits November 2, 2021 10:41
@promag
qml: Use window color instead of background
cd5527f 
Follow Qt recomendation (1) since the background is a solid color.

1. 
https://doc.qt.io/qt-5/qml-qtquick-controls2-applicationwindow.html#background-prop
@promag
qml: Remove explicit height of BlockCounter
6bedd3d 
Label implicit height depends on the font size but the font size depends
on the label height. This results in a runtime anchor loop warning.
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 7, 2025
@promag @hebasto
qml: Remove explicit height of BlockCounter
8e8047a 
Label implicit height depends on the font size but the font size depends
on the label height. This results in a runtime anchor loop warning.

Github-Pull: bitcoin-core#67
Rebased-From: 6bedd3d
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 7, 2025
@promag @hebasto
qml: Drop import qualifier BitcoinCoreComponents
052f88d 
Github-Pull: bitcoin-core#67
Rebased-From: 5b00612
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 7, 2025
@promag @hebasto
qml: Use ColumnLayout in application window
57d3a1e 
Github-Pull: bitcoin-core#67
Rebased-From: 37cfe4e
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 7, 2025
@promag @hebasto
qml: Add OptionButton control
15e52f0 
Github-Pull: bitcoin-core#67
Rebased-From: cbe91f6
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 7, 2025
@promag @hebasto
qml: Add ConnectionOptions component
b15011b 
Github-Pull: bitcoin-core#67
Rebased-From: 7985aad
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 7, 2025
@promag @hebasto
qml: Show ConnectionOptions
f158923 
Github-Pull: bitcoin-core#67
Rebased-From: 61668e9
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 8, 2025
@promag @hebasto
qml: Use window color instead of background
3188b3d 
Follow Qt recomendation (1) since the background is a solid color.

1.
https://doc.qt.io/qt-5/qml-qtquick-controls2-applicationwindow.html#background-prop

Github-Pull: bitcoin-core#67
Rebased-From: cd5527f
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 8, 2025
@promag @hebasto
qml: Remove explicit height of BlockCounter
5260afa 
Label implicit height depends on the font size but the font size depends
on the label height. This results in a runtime anchor loop warning.

Github-Pull: bitcoin-core#67
Rebased-From: 6bedd3d
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 8, 2025
@promag @hebasto
qml: Use ColumnLayout in application window
2bb4ba0 
Github-Pull: bitcoin-core#67
Rebased-From: 37cfe4e
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 8, 2025
@promag @hebasto
qml: Add OptionButton control
ee89acc 
Github-Pull: bitcoin-core#67
Rebased-From: cbe91f6
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 8, 2025
@promag @hebasto
qml: Add ConnectionOptions component
a67375b 
Github-Pull: bitcoin-core#67
Rebased-From: 7985aad
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 8, 2025
@promag @hebasto
qml: Show ConnectionOptions
1f832bd 
Github-Pull: bitcoin-core#67
Rebased-From: 61668e9
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 8, 2025
@promag @hebasto
qml: Use window color instead of background
a31f8f7 
Follow Qt recomendation (1) since the background is a solid color.

1.
https://doc.qt.io/qt-5/qml-qtquick-controls2-applicationwindow.html#background-prop

Github-Pull: bitcoin-core#67
Rebased-From: cd5527f
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 8, 2025
@promag @hebasto
qml: Remove explicit height of BlockCounter
aa73800 
Label implicit height depends on the font size but the font size depends
on the label height. This results in a runtime anchor loop warning.

Github-Pull: bitcoin-core#67
Rebased-From: 6bedd3d
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 8, 2025
@promag @hebasto
qml: Use ColumnLayout in application window
ed6b736 
Github-Pull: bitcoin-core#67
Rebased-From: 37cfe4e
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 8, 2025
@promag @hebasto
qml: Add OptionButton control
c7a704a 
Github-Pull: bitcoin-core#67
Rebased-From: cbe91f6
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 8, 2025
@promag @hebasto
qml: Add ConnectionOptions component
9aeb52d 
Github-Pull: bitcoin-core#67
Rebased-From: 7985aad
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 8, 2025
@promag @hebasto
qml: Show ConnectionOptions
d7e083d 
Github-Pull: bitcoin-core#67
Rebased-From: 61668e9
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
@promag @hebasto
qml: Use window color instead of background
3cc43b1 
Follow Qt recomendation (1) since the background is a solid color.

1.
https://doc.qt.io/qt-5/qml-qtquick-controls2-applicationwindow.html#background-prop

Github-Pull: bitcoin-core#67
Rebased-From: cd5527f
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
@promag @hebasto
qml: Remove explicit height of BlockCounter
fe1d5fe 
Label implicit height depends on the font size but the font size depends
on the label height. This results in a runtime anchor loop warning.

Github-Pull: bitcoin-core#67
Rebased-From: 6bedd3d
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
@promag @hebasto
qml: Use ColumnLayout in application window
e9c3510 
Github-Pull: bitcoin-core#67
Rebased-From: 37cfe4e
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
@promag @hebasto
qml: Add OptionButton control
3687a2f 
Github-Pull: bitcoin-core#67
Rebased-From: cbe91f6
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
@promag @hebasto
qml: Add ConnectionOptions component
f49076f 
Github-Pull: bitcoin-core#67
Rebased-From: 7985aad
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
@promag @hebasto
qml: Show ConnectionOptions
db9a901 
Github-Pull: bitcoin-core#67
Rebased-From: 61668e9
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
@promag @hebasto
qml: Use window color instead of background
13b85d4 
Follow Qt recomendation (1) since the background is a solid color.

1.
https://doc.qt.io/qt-5/qml-qtquick-controls2-applicationwindow.html#background-prop

Github-Pull: bitcoin-core#67
Rebased-From: cd5527f
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
@promag @hebasto
qml: Remove explicit height of BlockCounter
02eb58f 
Label implicit height depends on the font size but the font size depends
on the label height. This results in a runtime anchor loop warning.

Github-Pull: bitcoin-core#67
Rebased-From: 6bedd3d
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
@promag @hebasto
qml: Use ColumnLayout in application window
d50a52c 
Github-Pull: bitcoin-core#67
Rebased-From: 37cfe4e
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
@promag @hebasto
qml: Add OptionButton control
e3ccece 
Github-Pull: bitcoin-core#67
Rebased-From: cbe91f6
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
@promag @hebasto
qml: Add ConnectionOptions component
1e234dd 
Github-Pull: bitcoin-core#67
Rebased-From: 7985aad
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
@promag @hebasto
qml: Show ConnectionOptions
f04ed94 
Github-Pull: bitcoin-core#67
Rebased-From: 61668e9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jarolrod jarolrod jarolrod left review comments

@hebasto hebasto hebasto approved these changes

Assignees
No one assigned
Labels
UX Designers' opinions are required
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@promag @hebasto @Bosch-0 @jarolrod