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

examples: Add an example for scale functionality. #535

Closed
wants to merge 2 commits into from

Conversation

bratish
Copy link
Contributor

@bratish bratish commented Jul 27, 2021

Added a simple example for scale functionality.

Tested on Linux (Pop!_OS 21.04).

@@ -0,0 +1,108 @@
use gtk::prelude::*;
Copy link
Member

Choose a reason for hiding this comment

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

Please run cargo fmt on your code.

Comment on lines 56 to 67
horizontal_scale.connect_local(
"value-changed",
false,
move |args| {
let slider = args[0]
.get::<Scale>()
.expect("The value needs to be of type `Scale`.");

println!("Horizontal scale value: {:?}", slider.value());
None
})
.expect("Could not connect to signal.");
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
horizontal_scale.connect_local(
"value-changed",
false,
move |args| {
let slider = args[0]
.get::<Scale>()
.expect("The value needs to be of type `Scale`.");
println!("Horizontal scale value: {:?}", slider.value());
None
})
.expect("Could not connect to signal.");
horizontal_scale. connect_value_changed(move |slider {
println!("Horizontal scale value: {:?}", slider.value());
});

why not just?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe display those values on a label?

Comment on lines 77 to 88
vertical_scale.connect_local(
"value-changed",
false,
move |args| {
let slider = args[0]
.get::<Scale>()
.expect("The value needs to be of type `Scale`.");

println!("Vertical scale value: {:?}", slider.value());
None
})
.expect("Could not connect to signal.");
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
vertical_scale.connect_local(
"value-changed",
false,
move |args| {
let slider = args[0]
.get::<Scale>()
.expect("The value needs to be of type `Scale`.");
println!("Vertical scale value: {:?}", slider.value());
None
})
.expect("Could not connect to signal.");
vertical_scale.connect_local(
"value-changed",
false,
move |args| {
let slider = args[0]
.get::<Scale>()
.expect("The value needs to be of type `Scale`.");
println!("Vertical scale value: {:?}", slider.value());
None
})
.expect("Could not connect to signal.");

Same as the horizontal scale


fn main() {
let application = Application::new(
Some("com.github.gtk-rs.examples.builder_pattern"),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Some("com.github.gtk-rs.examples.builder_pattern"),
Some("com.github.gtk-rs.examples.scale"),

Comment on lines 2 to 9
use gtk::{ Adjustment,
Application,
ApplicationWindow,
Grid,
Label,
Orientation,
Scale
};
Copy link
Member

Choose a reason for hiding this comment

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

I definitely don't like importing everything here. In general, I would do that unless the type/function/macro is used more than once.

@@ -0,0 +1,5 @@
# Basics

This example demonstrates how to create `scales` and place them in the `window` using a grid. It also shows how to implement `callbacks`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
This example demonstrates how to create `scales` and place them in the `window` using a grid. It also shows how to implement `callbacks`.
This example demonstrates how to create `gtk::Scale` and place them in the `gtk::Window` using a grid.

Something like this?

@@ -0,0 +1,5 @@
# Basics
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Basics
# Scale

maybe?
This also needs to be added to the main readme inside the examples directory

@bilelmoussaoui
Copy link
Member

Thanks for the example! overall looks good to me. Do you mind taking the screenshot using the default GTK theme like the other examples?

@bratish
Copy link
Contributor Author

bratish commented Jul 28, 2021

Thank you for taking time and review the PR. I have tried to implement all the suggestions except the screenshot. The only computer I have access to is a Pop Os! and I could not change it to the theme to match the ones in the other examples (the default GTK theme). :(

Please let me know if this is okay. Thank you!

@bilelmoussaoui
Copy link
Member

Thank you for taking time and review the PR. I have tried to implement all the suggestions except the screenshot. The only computer I have access to is a Pop Os! and I could not change it to the theme to match the ones in the other examples (the default GTK theme). :(

Please let me know if this is okay. Thank you!

You can change the theme in the settings i believe and switch back once you took the screenshot. Sorry for the long delay, i forgot you actually replied.

@bilelmoussaoui
Copy link
Member

Rebased and fixed on #1569, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants