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

library: Adds ColorPicker Demo #335

Merged
merged 8 commits into from
Jun 17, 2023
Merged

library: Adds ColorPicker Demo #335

merged 8 commits into from
Jun 17, 2023

Conversation

SoNiC-HeRE
Copy link
Contributor

Screencast.from.2023-06-14.11-47-15.webm

Closes #334

Copy link
Contributor

@sonnyp sonnyp left a comment

Choose a reason for hiding this comment

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

You're not using the promisified version using await

Please refer to the Screenshot library entry.

@sonnyp sonnyp removed the request for review from andyholmes June 14, 2023 10:40
using Adw 1;

Adw.StatusPage {
title: "Color Picker";
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
title: "Color Picker";
title: _("Color Picker");

label: _("Select Color");
margin-bottom: 42;
halign: center;
styles ["suggested-action"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
styles ["suggested-action"]
styles ["suggested-action", "pill"]

}
}

button.connect("clicked", pickColor);
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to #330 (comment)

Remove the try/catch, rename and do

Suggested change
button.connect("clicked", pickColor);
button.connect("clicked", () => {
onClicked().catch(logError);
});


async function pickColor() {
try {
const color = await portal.pick_color(parent, null);
Copy link
Contributor

Choose a reason for hiding this comment

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

the result of pick_color isn't really a color, I would naming the return value result instead and move the comment there

Suggested change
const color = await portal.pick_color(parent, null);
// result is a GVariant of the form (ddd), containing red, green and blue components in the range [0,1]
const result = await portal.pick_color(parent, null);

Comment on lines 17 to 20
const colorInfo = color.print(false);

// The picked color is a GVariant of the form (ddd), containing red, green and blue components in the range [0,1].
console.log(`Selected Color is: ${colorInfo}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

wdyt we create a rgba color with https://docs.gtk.org/gdk4/struct.RGBA.html and then use color.to_string() ?

similar to the Color Dialog entry
You can read rgb from result using const [r,g,b] = result.deepUnpack()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

@SoNiC-HeRE
Copy link
Contributor Author

Made the following changes:

  • Removed async handler
  • Refined Code
  • Used Gdk.RGBA for better output

@SoNiC-HeRE SoNiC-HeRE requested a review from sonnyp June 16, 2023 16:52
bazylevnik0 and others added 4 commits June 17, 2023 19:07
* libary: Add SpinButton entry
* Spin Button: Add link to tutorial
* Spin Button: Changes from review
* Spin Button: Translated strings
* Spin Button: Add comments for clarity
* library: Add AdwBanner entry
* Banner: Translate strings
Copy link
Contributor

@sonnyp sonnyp left a comment

Choose a reason for hiding this comment

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

Well done ✨

@sonnyp sonnyp merged commit d43b6e7 into main Jun 17, 2023
@sonnyp sonnyp deleted the sonic/colorpicker branch June 17, 2023 17:11
sonnyp pushed a commit that referenced this pull request Jun 20, 2023
sonnyp pushed a commit to SoNiC-HeRE/Workbench that referenced this pull request Aug 13, 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.

Add ColorPicker Demo
4 participants