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

Add NavButton interaction color animation states #234

Merged
merged 2 commits into from
Jan 29, 2023

Conversation

jarolrod
Copy link
Member

Introduces the missing NavButton interaction color animation states as detailed in #231, except there are issues with the sizing of the components themselves.

Dark Mode

Default Hovered Pressed
Screen Shot 2023-01-29 at 4 41 48 AM Screen Shot 2023-01-29 at 4 42 04 AM Screen Shot 2023-01-29 at 4 42 21 AM

Dark Mode

Default Hovered Pressed
Screen Shot 2023-01-29 at 4 43 40 AM Screen Shot 2023-01-29 at 4 44 00 AM Screen Shot 2023-01-29 at 4 44 12 AM

As can be told from the screenshots the sizing of the component itself isn't correct; this needs to be addressed, potentially in a follow-up.

The first commit allows for us to use the NavButton correctly with these new background states but also fixes an issue where the Right Caret buttons in the NodeSettings page were much bigger than in the other areas we use this button. A follow-up can contain this button in a nice component.

Windows
Intel macOS
Apple Silicon macOS
ARM64 Android

@johnny9
Copy link
Contributor

johnny9 commented Jan 29, 2023

ACK 20edc75

Buttons behave as expected and no QML errors appear. I think this is an improvement over the flat buttons with no state.

As can be told from the screenshots the sizing of the component itself isn't correct; this needs to be addressed, potentially in a follow-up.

Fixing the size of these buttons will help with the focus state as well. I tried adjusting for it but it wasn't a great solution so I agree its best to just approach it as another issue/PR.

@hebasto hebasto merged commit f3150f7 into bitcoin-core:main Jan 29, 2023
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