library: Add Scale entry #204

SoNiC-HeRE · 2023-03-03T17:21:49Z

Added Scale Entry under User Interface Section in Library
Issue workbenchdev/demos#3

sonnyp

I think we should add a vertical example and have one of them with draw-value: true

Let's use the UI panel whenever possible. I know it's not obvious but you can do most of this directly from there , hopefully Workbench will make that more obvious to new developers.

example:

    Scale one {
      orientation: horizontal;
      margin-bottom: 20;
      adjustment: Gtk.Adjustment {
        lower: 0;
        upper: 100;
        value: 70;
      };
    }

sonnyp · 2023-03-05T10:29:33Z

src/Library/demos/Scale/main.blp

+
+Adw.StatusPage {
+  title: "Scale";
+  description: _("A simple sliding control");


Since it's not a very common control I think we can be a bit more descriptive

Suggested change

description: _("A simple sliding control");

description: _("Slider control to select a value from a range");

sonnyp · 2023-03-05T10:39:53Z

src/Library/demos/Scale/main.blp

+    LinkButton {
+      label: "Tutoral";
+      uri: "https://developer-old.gnome.org/gtkmm-tutorial/stable/sec-scale-widgets.html.en";
+    }


if the current tutorial doesn't have an entry for slider - let's remove this link

sonnyp · 2023-03-05T10:44:10Z

src/Library/demos/Scale/main.js

+
+scale_two.set_range(true, 50);
+scale_two.set_inverted(true);
+scale_two.add_mark(10.0, "right", null);


"right" doesn't work

also since this is a horizontal slider - let's use bottom

it should be Gtk.PositionType.BOTTOM for the second argument

sonnyp · 2023-03-05T10:46:45Z

src/Library/demos/Scale/main.js

+scale_two.add_mark(20.0, "right", null);
+scale_two.add_mark(30.0, "right", null);
+scale_two.add_mark(40.0, "right", null);
+scale_two.add_mark(50.0, "right", null);


I think 2 or 3 marks are enough

can you add labels to them? I'd suggest letters "A", "B", ...

Okay, I'll add 2 marks with labels

sonnyp · 2023-03-05T10:47:10Z

src/Library/demos/Scale/main.js

+scale_disabled.set_value(25);
+scale_disabled.set_show_fill_level(25);
+
+scale_one.connect("value_changed", () => {


Suggested change

scale_one.connect("value_changed", () => {

scale_one.connect("value-changed", () => {

sonnyp · 2023-03-05T10:56:05Z

src/Library/demos/Scale/main.js

+const scale_two = workbench.builder.get_object("two");
+const scale_disabled = workbench.builder.get_object("disabled");
+
+scale_one.set_range(true, 50);


set_range takes 2 numbers

sonnyp · 2023-03-05T10:56:41Z

src/Library/demos/Scale/main.js

+scale_one.set_range(true, 50);
+
+scale_two.set_range(true, 50);
+scale_two.set_inverted(true);


invertedis verty unusual - I think we can leave it out

sonnyp · 2023-03-05T10:58:54Z

src/Library/demos/Scale/main.js

+  if (scale_value === 50) {
+    console.log("Max Value Reached");
+  } else if (scale_value === 1) {
+    console.log("Min Value Reached");
+  }


this is a good idea but instead of repeating please retrieve the min and max value directly from the object

you can do so with scale_one.adjustment.lower and upper

sonnyp · 2023-03-05T10:59:39Z

src/Library/demos/Scale/main.js

+scale_two.connect("value_changed", () => {
+  let scale_value_two = scale_two.get_value();
+  if (scale_value_two === 50) {
+    console.log("Max Value Reached");
+  } else if (scale_value_two === 1) {
+    console.log("Min Value Reached");
+  }
+});


I don't think we need the same example a second time

maybe print the current value instead

SoNiC-HeRE · 2023-03-06T19:42:45Z

Applied the Changes as Suggested , Implemented a Vertical Slider and stuck to a 3 slider layout for cleaner UI. Implemented 4 marks on the Vertical Slider and Updated JS Code

sonnyp

not building/working

please review your own changes before submitting for review

sonnyp · 2023-03-07T22:05:05Z

src/Library/demos/Scale/main.js

+scale_two.add_mark(100.0, "right", "D");
+scale_two.set_increments(25.0, 25.0);
+
+scale_disabled.set_show_fill_level(25);


set_show_fill_level takes a boolean, not an integer
makes sure that you're using the APIs correctly—not just that things work

also show-fille-level is a property so it can be moved to main.blp if needed

okay, I'll move it to main.blp

sonnyp · 2023-03-07T22:06:01Z

src/Library/demos/Scale/main.js

+scale_one.connect("value-changed", () => {
+  let scale_value = scale_one.get_value();
+  if (scale_value === scale_one.adjustment.upper) {
+    console.log("Max Value Reached");


Suggested change

console.log("Max Value Reached");

console.log("Maximum value reached");

sonnyp · 2023-03-07T22:06:24Z

src/Library/demos/Scale/main.js

+  if (scale_value === scale_one.adjustment.upper) {
+    console.log("Max Value Reached");
+  } else if (scale_value === scale_one.adjustment.lower) {
+    console.log("Min Value Reached");


Suggested change

console.log("Min Value Reached");

console.log("Minimum value reached");

sonnyp · 2023-03-07T22:11:54Z

src/Library/demos/Scale/main.js

+scale_two.add_mark(25.0, "left", "A");
+scale_two.add_mark(50.0, "right", "B");
+scale_two.add_mark(75.0, "right", "C");
+scale_two.add_mark(100.0, "right", "D");


ha yes - Blueprint doesn't support marks unfortunaly so we have to do it in JS

https://gitlab.gnome.org/jwestman/blueprint-compiler/-/issues/62

It's already in JS. So shall i leave it unchanged?

yes - it's good as is

sonnyp · 2023-03-07T22:14:30Z

src/Library/demos/Scale/main.js

+scale_two.connect("value-changed", () => {
+  let scale_value_two = scale_two.get_value();
+  if (scale_value_two === 25) {
+    console.log("Mark A Reached");
+  } else if (scale_value_two === 50) {
+    console.log("Mark B Reached");
+  } else if (scale_value_two === 75) {
+    console.log("Mark C Reached");
+  } else if (scale_value_two === 100) {
+    console.log("Mark D Reached");
+  }
+});


Instead of repeating the mark values - please create them and find them from the same values.

For example - store the marks in an object and use that to add the marks and find the label in value-chaged

sonnyp · 2023-03-07T22:16:48Z

src/Library/demos/Scale/main.blp

+
+        Scale one {
+          orientation: horizontal;
+          margin-bottom: 20;


please use multiples of 6 for margins paddings and so on

it's the GNOME guideline

6, 12, 18, 24, etc

SoNiC-HeRE · 2023-03-08T12:38:43Z

Applied Suggested Changes: Updated JS Code and implemented Changes in Blp File as well.

sonnyp

Thanks.

Regarding marks, this is what I meant: 4f8e9c5

sonnyp requested changes Mar 5, 2023

View reviewed changes

sonnyp added the Library 📚️ label Mar 6, 2023

Applied Changes

8d7e515

SoNiC-HeRE force-pushed the Scale branch from b53212e to 8d7e515 Compare March 6, 2023 19:30

Updated Third Slider

31ea2b3

SoNiC-HeRE requested a review from sonnyp March 6, 2023 19:42

sonnyp requested changes Mar 7, 2023

View reviewed changes

Updated Scale Entry

62e25b9

sonnyp requested changes Mar 7, 2023

View reviewed changes

sonnyp reviewed Mar 7, 2023

View reviewed changes

Applied Suggested Changes

3ba4ba3

sonnyp added 3 commits March 8, 2023 14:26

Merge branch 'main' into Scale

3b9858a

improve code

4f8e9c5

fix formatting

17685f8

sonnyp approved these changes Mar 8, 2023

View reviewed changes

sonnyp merged commit 59e12f2 into workbenchdev:main Mar 8, 2023

sonnyp pushed a commit to SoNiC-HeRE/Workbench that referenced this pull request Mar 13, 2023

library: Add Scale entry (workbenchdev#204)

146647a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

library: Add Scale entry #204

library: Add Scale entry #204

SoNiC-HeRE commented Mar 3, 2023

sonnyp left a comment

sonnyp Mar 5, 2023

SoNiC-HeRE Mar 5, 2023

sonnyp Mar 5, 2023

sonnyp Mar 5, 2023

sonnyp Mar 5, 2023

SoNiC-HeRE Mar 5, 2023

sonnyp Mar 5, 2023

sonnyp Mar 5, 2023

sonnyp Mar 5, 2023

SoNiC-HeRE Mar 5, 2023

sonnyp Mar 5, 2023

sonnyp Mar 5, 2023

SoNiC-HeRE Mar 5, 2023

SoNiC-HeRE commented Mar 6, 2023

sonnyp left a comment

sonnyp Mar 7, 2023

SoNiC-HeRE Mar 7, 2023

sonnyp Mar 7, 2023

sonnyp Mar 7, 2023

sonnyp Mar 7, 2023

SoNiC-HeRE Mar 7, 2023

sonnyp Mar 8, 2023

sonnyp Mar 7, 2023

sonnyp Mar 7, 2023 •

edited

SoNiC-HeRE commented Mar 8, 2023

sonnyp left a comment

	description: _("A simple sliding control");
	description: _("Slider control to select a value from a range");

	scale_one.connect("value_changed", () => {
	scale_one.connect("value-changed", () => {

	console.log("Max Value Reached");
	console.log("Maximum value reached");

	console.log("Min Value Reached");
	console.log("Minimum value reached");

library: Add Scale entry #204

library: Add Scale entry #204

Conversation

SoNiC-HeRE commented Mar 3, 2023

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

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 6, 2023

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonnyp Mar 7, 2023 • edited

Choose a reason for hiding this comment

SoNiC-HeRE commented Mar 8, 2023

sonnyp left a comment

Choose a reason for hiding this comment

sonnyp Mar 7, 2023 •

edited