Skip to content

Conversation

@johnny9
Copy link
Collaborator

@johnny9 johnny9 commented Feb 18, 2023

Windows
Intel macOS
Apple Silicon macOS
ARM64 Android

@johnny9 johnny9 force-pushed the peers branch 3 times, most recently from 2ca4ba1 to 8ab488f Compare February 18, 2023 23:02
@johnny9 johnny9 changed the title qml: Add sorting bar to Peers page Add sorting bar to Peers page Feb 19, 2023
@johnny9 johnny9 force-pushed the peers branch 2 times, most recently from 159bc04 to c059cf3 Compare February 19, 2023 04:22
@johnny9
Copy link
Collaborator Author

johnny9 commented Feb 19, 2023

Updates from 8ab488f to c059cf3:

  • Fix spacing between ToggleButtons
  • Fix vertical spacing for ListView items
  • Make "ID" checked by default

@johnny9
Copy link
Collaborator Author

johnny9 commented Feb 19, 2023

ID Direction User Agent Type Ip Network
Screenshot from 2023-02-18 23-20-50 Screenshot from 2023-02-18 23-21-06 Screenshot from 2023-02-18 23-21-14 Screenshot from 2023-02-18 23-21-21 Screenshot from 2023-02-18 23-21-28 Screenshot from 2023-02-18 23-21-35

Copy link
Contributor

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

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

toggle button text color doesn't work on light mode

Screen Shot 2023-02-20 at 6 51 59 PM

Copy link
Contributor

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

Diff on ToggleButton.qml

diff --git a/src/qml/controls/ToggleButton.qml b/src/qml/controls/ToggleButton.qml
index 05cf54e6a..ea72c5802 100644
--- a/src/qml/controls/ToggleButton.qml
+++ b/src/qml/controls/ToggleButton.qml
@@ -6,38 +6,30 @@ import QtQuick 2.15
 import QtQuick.Controls 2.15

 Button {
-    property color defaultColor: Theme.color.neutral2
-    property color hoverColor: Theme.color.neutral2
-    property color activeColor: Theme.color.neutral5
+    id: root
+    property color bgDefaultColor: Theme.color.neutral2
+    property color bgHoverColor: Theme.color.neutral2
+    property color bgActiveColor: Theme.color.neutral5
     property color textColor: Theme.color.neutral7
     property color textHoverColor: Theme.color.orangeLight1
     property color textActiveColor: Theme.color.neutral9
-    id: root
-    checkable: true
     hoverEnabled: true
-    font.family: "Inter"
-    font.styleName: "Regular"
-    font.pixelSize: 13
+    checkable: true
     leftPadding: 12
     rightPadding: 12
     topPadding: 5
     bottomPadding: 5

-    contentItem: Text {
+    contentItem: Header {
         id: buttonText
-        text: parent.text
-        font: parent.font
-        color: Theme.color.white
-        horizontalAlignment: Text.AlignHCenter
-        verticalAlignment: Text.AlignVCenter
-
-        Behavior on color {
-            ColorAnimation { duration: 150 }
-        }
+        header: root.text
+        headerSize: 13
+        headerColor: root.textColor
     }
+
     background: Rectangle {
         id: bg
-        color: root.defaultColor
+        color: root.bgDefaultColor
         radius: 5

         Behavior on color {
@@ -45,24 +37,22 @@ Button {
         }
     }

-    state:"DEFAULT"
+    state: "DEFAULT"

     states: [
         State {
             name: "DEFAULT"
-            PropertyChanges { target: bg; color: root.defaultColor }
+            PropertyChanges { target: bg; color: root.bgDefaultColor }
         },
         State {
-            name: "CHECKED"
-            when: root.checked
-            PropertyChanges { target: bg; color: root.activeColor }
-            PropertyChanges { target: buttonText; color: root.textActiveColor }
+            name: "CHECKED"; when: root.checked
+            PropertyChanges { target: bg; color: root.bgActiveColor }
+            PropertyChanges { target: buttonText; headerColor: root.textActiveColor }
         },
         State {
-            name: "HOVER"
-            when: root.hovered
-            PropertyChanges { target: bg; color: root.hoverColor }
-            PropertyChanges { target: buttonText; color: root.textHoverColor }
+            name: "HOVER"; when: root.hovered
+            PropertyChanges { target: bg; color: root.bgHoverColor }
+            PropertyChanges { target: buttonText; headerColor: root.textHoverColor }
         }
     ]
 }

diff on Peers.qml

diff --git a/src/qml/pages/node/Peers.qml b/src/qml/pages/node/Peers.qml
index 2d9b8b2af..546bbc6b2 100644
--- a/src/qml/pages/node/Peers.qml
+++ b/src/qml/pages/node/Peers.qml
@@ -30,11 +30,12 @@ Page {
         horizontalAlignment: Text.AlignHCenter
     }

-    RowLayout {
+    Flow {
         id: sortSelection
         anchors.top: description.bottom
         anchors.topMargin: 20
         anchors.horizontalCenter: parent.horizontalCenter
+        width: Math.min(parent.width - 40, 450)
         spacing: 10
         ToggleButton {
             text: qsTr("ID")

@johnny9 johnny9 force-pushed the peers branch 4 times, most recently from e1cc8d0 to b36b8d5 Compare February 22, 2023 04:41
@johnny9
Copy link
Collaborator Author

johnny9 commented Feb 22, 2023

from c059cf3 to b36b8d5 :

  • fixed togglebutton text color in light mode
  • used CoreText to reduce duplication of Text font declarations
  • removed explicit DEFAULT state
  • prefix togglebutton background color properties with bg
  • inline when statements
  • Use Flow to wrap toggle button row

@johnny9
Copy link
Collaborator Author

johnny9 commented Feb 22, 2023

Above fixes for lightmode and togglebutton wrapping:
Screenshot from 2023-02-21 23-44-00

Copy link
Contributor

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

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

The CoreText component needs review on its own. Can you move these changes out to its own independent PR?

PR master
Screen Shot 2023-02-22 at 3 08 52 AM Screen Shot 2023-02-22 at 3 10 08 AM

@hebasto
Copy link
Member

hebasto commented Feb 22, 2023

Rebase?

@johnny9
Copy link
Collaborator Author

johnny9 commented Feb 22, 2023

The CoreText component needs review on its own. Can you move these changes out to its own independent PR?
PR master
Screen Shot 2023-02-22 at 3 08 52 AM Screen Shot 2023-02-22 at 3 10 08 AM

#274
Interesting that its wrapping for you. I'm not seeing that behavior. Ill try on the platforms I have. Might be macOS.

@hebasto
Copy link
Member

hebasto commented Feb 27, 2023

Needs rebase again :)

@johnny9
Copy link
Collaborator Author

johnny9 commented Mar 1, 2023

Update from 51a3e21 to a46d12d:

  • rebased with main

Copy link
Contributor

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

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

ACK a46d12d

One thing to note is I believe that the toggle buttons have three states, selected, selected again, not selected. When selected again it should reverse the sorting order; so if selected is least to greatest, selected again would be greatest to least. We can connect with christoph about this soon.

@hebasto hebasto merged commit bc10f64 into bitcoin-core:main Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants