library: Add Toggle Button Entry by SoNiC-HeRE · Pull Request #219 · workbenchdev/Workbench

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

+      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

+        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

+    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

+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

+  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

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


this id is not used

also - lower case only for ids

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

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


Suggested change

label: "Grouped Toggle Buttons";

label: "Grouped";

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

+
+    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

+    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

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


use spacing on the parent indead

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

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


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

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

+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

+  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

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

+      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

Uh oh!

Conversation

SoNiC-HeRE commented Mar 11, 2023

Uh oh!

SoNiC-HeRE commented Mar 11, 2023

Uh oh!

bertob commented Mar 13, 2023

Uh oh!

sonnyp commented Mar 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SoNiC-HeRE commented Mar 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonnyp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SoNiC-HeRE commented Mar 14, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SoNiC-HeRE commented Mar 19, 2023

Uh oh!

sonnyp left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sonnyp commented Mar 13, 2023 •

edited

Loading

SoNiC-HeRE commented Mar 13, 2023 •

edited

Loading