library: Add Toggle Button Entry #219

SoNiC-HeRE · 2023-03-11T12:46:51Z

Added Toggle Button Entry/Demo under User Interface section in Library.
Issue workbenchdev/demos#3

SoNiC-HeRE · 2023-03-11T12:52:36Z

Kept This Entry minimalistic for a clean design, Implemented icons instead of labels for a nicer feel and look.
Divided into 3 sections : 1) Grouped Toggle Buttons 2) Ungrouped Toggle Buttons 3) Disabled Toggle Button.
Could've implemented icon-change design (when a button is toggled) for a better look but probably wont help with grouped Toggle Buttons. Used Icons with more day to day use so that user can relate.

bertob · 2023-03-13T08:40:10Z

The wide "home" button is a bit weird, what exactly are you trying to demo there? Width-request? Sensitive? My feeling is that button could be dropped, or if you want to make it more unique it could be a button with icon+label. Not sure that belongs in this demo vs. the button one though...

Also, I'd not use the two volume icons next to each other, that implies some kind of audio functionality at first glance. I'd use some other icon as the second one, e.g. list-compact or some other generic symbol.

sonnyp · 2023-03-13T08:42:26Z

Maybe replace the home button with a text toggle button (not disabled).

Or as Tobias said a button + icon 👍 https://gnome.pages.gitlab.gnome.org/libadwaita/doc/main/class.ButtonContent.html

SoNiC-HeRE · 2023-03-13T12:37:03Z

I tried implementing some sort of logic as we may get in discord ,i.e, deafen and undeafen audio so i thought it was a good example for the grouped toggle buttons where one is active the other automatically gets untoggled. But as you said I'll implement the changes for the last toggle button and the second toggle button as well

sonnyp

Please make this look a bit better

sonnyp · 2023-03-14T12:20:10Z

src/Library/demos/Toggle Button/main.blp

+      ToggleButton button_first {
+        active: true;
+        icon-name: "audio-volume-muted-symbolic";
+        halign: center;
+        margin-end: 12;
+        margin-bottom: 18;
+      }
+
+      ToggleButton button_second {
+        active: false;
+        icon-name: "list-compact";
+        halign: center;
+        group: button_first;
+        margin-bottom: 18;
+      }


ha it's a group then you should use "linked" on the parent box and no space between

please also add a label to explain what each example is about, here "Group"

For the icons - maybe best to use something with a clear on/off state like "eye-not-looking-symbolic" and "eye-open-negative-filled-symbolic"

If you move margin-bottom: 18; to the parent - it doesn't need to be repeated

sonnyp · 2023-03-14T12:26:55Z

src/Library/demos/Toggle Button/main.blp

+        label: "Console";
+        icon-name: "code-symbolic";


Suggested change

label: "Console";

icon-name: "code-symbolic";

label: "Console";

icon-name: "terminal-symbolic";

sonnyp · 2023-03-14T12:27:35Z

src/Library/demos/Toggle Button/main.blp

+    ToggleButton disabled {
+      halign: center;
+      margin-bottom: 18;
+      sensitive: false;


Suggested change

ToggleButton disabled {

halign: center;

margin-bottom: 18;

sensitive: false;

ToggleButton {

halign: center;

margin-bottom: 18;

a disabled example isn't helpful here because it's the same as a regular disabled button

sonnyp · 2023-03-14T12:29:07Z

src/Library/demos/Toggle Button/main.js

+third.connect("toggled", () => {
+  toggle_count_camera++;
+  console.log(
+    toggle_count_camera % 2 !== 0 ? "Camera Toggled On" : "Camera Toggled Off",


use the active property instead

also, for the text, "Camera on" and "Camera off"

sonnyp · 2023-03-14T12:29:14Z

src/Library/demos/Toggle Button/main.js

+  toggle_count_flashlight++;
+  console.log(
+    toggle_count_flashlight % 2 !== 0
+      ? "Flashlight Toggled On"
+      : "Flashlight Toggled Off",


SoNiC-HeRE · 2023-03-14T14:29:27Z

Sure, I'll make the changes.

sonnyp · 2023-03-18T23:20:50Z

src/Library/demos/Toggle Button/main.blp

+  Box {
+    orientation: vertical;
+
+    Box Grouped_Toggle_Buttons {


this id is not used

also - lower case only for ids

sonnyp · 2023-03-18T23:21:44Z

src/Library/demos/Toggle Button/main.blp

+    }
+
+    Label {
+      label: "Grouped Toggle Buttons";


Suggested change

label: "Grouped Toggle Buttons";

label: "Grouped";

sonnyp · 2023-03-18T23:21:54Z

src/Library/demos/Toggle Button/main.blp

+
+    Label {
+      label: "Grouped Toggle Buttons";
+      justify: center;


this does nothing
in general - remove properties which don't do anything (unless we need them for examples)

sonnyp · 2023-03-18T23:22:27Z

src/Library/demos/Toggle Button/main.blp

+    Box Grouped_Toggle_Buttons {
+      orientation: horizontal;
+      halign: center;
+      margin-bottom: 12;


please use 6px spacing for things that are relevant to each others like the label and its content

sonnyp · 2023-03-18T23:23:11Z

src/Library/demos/Toggle Button/main.blp

+        active: false;
+        icon-name: "photo-camera-symbolic";
+        halign: center;
+        margin-end: 6;


use spacing on the parent indead

sonnyp · 2023-03-18T23:24:41Z

src/Library/demos/Toggle Button/main.blp

+        label: "Console";
+        icon-name: "terminal-symbolic";
+      }
+    }


let's add a label to this one as well - "With label"

sonnyp · 2023-03-18T23:25:15Z

src/Library/demos/Toggle Button/main.js

+const terminal = workbench.builder.get_object("console");
+
+camera.connect("toggled", () => {
+  console.log(camera.get_active() ? "Camera On" : "Camera Off");


as I said before - use properties when possible - not methods

sonnyp · 2023-03-18T23:26:38Z

src/Library/demos/Toggle Button/main.js

+  console.log(flashlight.get_active() ? "Flashlight On" : "Flashlight Off");
+});
+terminal.connect("toggled", () => {
+  console.log(terminal.get_active() ? "Entered Console" : "Exited Console");


I'd use On / Off here too

sonnyp · 2023-03-18T23:27:12Z

src/Library/demos/Toggle Button/main.js

@@ -0,0 +1,13 @@
+const camera = workbench.builder.get_object("button_third");
+const flashlight = workbench.builder.get_object("button_fourth");
+const terminal = workbench.builder.get_object("console");


there should be a Code demo to get the current selected button from the group

sonnyp · 2023-03-18T23:27:31Z

src/Library/demos/Toggle Button/main.blp

+      ToggleButton button_second {
+        active: false;
+        icon-name: "eye-open-negative-filled-symbolic";
+        group: button_first;


there should be a Code demo to get the current selected button from the group

SoNiC-HeRE · 2023-03-19T06:51:43Z

I'll make the required changes

sonnyp

Thanks!

I made a small improvemets 44d9ec8

Mostly increase spacing between the various "modes" so everything doesn't look pack and to clarify what the label describes.

Simplified the JS a bit and used notify::active which is the right way to listen for change son the active property.

Add Toggle Button entry

223090d

SoNiC-HeRE changed the title ~~library:Add Toggle Button Entry~~ library: Add Toggle Button Entry Mar 11, 2023

sonnyp added the Library 📚️ label Mar 11, 2023

sonnyp requested a review from bertob March 11, 2023 14:04

sonnyp assigned bertob Mar 11, 2023

sonnyp mentioned this pull request Mar 12, 2023

library: Add Box entry #216

Merged

Applied Suggested Changes

a2b2318

sonnyp reviewed Mar 14, 2023

View reviewed changes

SoNiC-HeRE added 2 commits March 15, 2023 01:44

Applied Suggested Changes

c82daa8

Improved JS Code

a807b25

sonnyp assigned sonnyp and unassigned bertob Mar 16, 2023

sonnyp requested changes Mar 18, 2023

View reviewed changes

Improved UI and JS code

a036259

SoNiC-HeRE requested review from sonnyp and removed request for bertob March 21, 2023 12:07

sonnyp added 2 commits March 21, 2023 16:19

Merge branch 'main' into Toggle-Button

39b2caa

small improvements

44d9ec8

sonnyp approved these changes Mar 21, 2023

View reviewed changes

sonnyp merged commit 8e2c8df into workbenchdev:main Mar 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

library: Add Toggle Button Entry #219

library: Add Toggle Button Entry #219

SoNiC-HeRE commented Mar 11, 2023

SoNiC-HeRE commented Mar 11, 2023

bertob commented Mar 13, 2023

sonnyp commented Mar 13, 2023 •

edited

SoNiC-HeRE commented Mar 13, 2023 •

edited

sonnyp left a comment

sonnyp Mar 14, 2023

sonnyp Mar 14, 2023

sonnyp Mar 14, 2023

sonnyp Mar 14, 2023

sonnyp Mar 14, 2023

SoNiC-HeRE commented Mar 14, 2023

sonnyp Mar 18, 2023

sonnyp Mar 18, 2023

sonnyp Mar 18, 2023

sonnyp Mar 18, 2023

sonnyp Mar 18, 2023

sonnyp Mar 18, 2023

sonnyp Mar 18, 2023

sonnyp Mar 18, 2023

sonnyp Mar 18, 2023

sonnyp Mar 18, 2023

SoNiC-HeRE commented Mar 19, 2023

sonnyp left a comment

library: Add Toggle Button Entry #219

library: Add Toggle Button Entry #219

Conversation

SoNiC-HeRE commented Mar 11, 2023

SoNiC-HeRE commented Mar 11, 2023

bertob commented Mar 13, 2023

sonnyp commented Mar 13, 2023 • edited

SoNiC-HeRE commented Mar 13, 2023 • edited

sonnyp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SoNiC-HeRE commented Mar 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SoNiC-HeRE commented Mar 19, 2023

sonnyp left a comment

Choose a reason for hiding this comment

sonnyp commented Mar 13, 2023 •

edited

SoNiC-HeRE commented Mar 13, 2023 •

edited