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.

Sign up for GitHub

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

Jump to bottom

Wayland colorpicking #619

Closed
wants to merge 19 commits into from
Closed

Wayland colorpicking #619

wants to merge 19 commits into from

Conversation

madjesc
Copy link

@madjesc madjesc commented Jun 28, 2021 •
edited
Loading

Summary / How this PR fixes the problem?

Implementing colorpicking in Wayland using libportal

Steps to Test

Screenshots

image
image

Known Issues / Things To Do

  • Handle pick_color_finish exception
  • Test in Xorg

This PR fixes/implements the following bugs/features:

@madjesc
Copy link
Author

madjesc commented Jun 28, 2021

It not works with the master update, fixing

bilelmoussaoui
bilelmoussaoui reviewed Jun 28, 2021
View reviewed changes
meson.build Outdated Show resolved Hide resolved
Alecaddd and others added 7 commits June 28, 2021 15:45
@Alecaddd @madjesc
Implement global colors (akiraux#599)
7120c73 
* Create initial widgets and settings for global colors

* Style color library

* Improve styling and remove duplication
@Alecaddd @madjesc
Partials widgets rename (akiraux#600)
3a5ee74 
* Rename Partials to Widgets

* Consistent block comment for all widgets

* Disconnect events on destruction

* Remove constructor and update window attribute to unowned

* Update the ButtonImage widget
@Alecaddd @madjesc
Update README.md
e69ed33
@Alecaddd @madjesc
Add color row widget (akiraux#604)
5bf10e4 
* Move color picker code to its own class

* Move color field inside reusable color picker widget

* Move the opacity field in the color button widget

* Update the item fill color when the opacity field is changed

* Properly update colors and alpha of items and UI buttons

* Use a shared ColorModel for fills and borders

* Reload list of fill deletion

* Remove 2 way bindings for color fields

* Hook border to reusable UI widget
@Alecaddd @madjesc
Fix nobs regression (akiraux#608)
5401218 
* Fix regression of corner nobs not shifting when the item is small

* Do early return for rotation nob before other conditions

* Account for item rotation. Fixes akiraux#607
@Sav22999 @madjesc
Translated in Italian (akiraux#618)
e339e45 
* Translated in Italian

* Fixed some typos
@madjesc
Color picker working under wayland
51268af
@madjesc
Copy link
Author

madjesc commented Jun 28, 2021

Now it works, using libportal 4.0
And now the Utils.ColorPicker class is not a window,
is a class that emmits a signal when the colorpicking is done.

@Alecaddd
Copy link
Member

Alecaddd commented Jul 4, 2021

@Mkefs Hi, thank you so much for your contribution.
This is great, but indeed it shouldn't break this feature for X11 users, which are our primary target for now.
Any chance of making this work for both?

@madjesc
Copy link
Author

madjesc commented Jul 12, 2021

@Mkefs Hi, thank you so much for your contribution.
This is great, but indeed it shouldn't break this feature for X11 users, which are our primary target for now.
Any chance of making this work for both?

Ok, I'll try

@Alecaddd Alecaddd marked this pull request as draft July 14, 2021 03:35
@Alecaddd
Copy link
Member

Converting this to a Draft PR since it's not ready for a full review or to be merged.

@madjesc
Copy link
Author

madjesc commented Jul 15, 2021

It works in Xorg.
Tested on Gnome 3.38 on Debian bullseye
imagen

@Alecaddd Alecaddd marked this pull request as ready for review July 15, 2021 20:50
@Alecaddd
Copy link
Member

Awesome, I'll review this later today.
Thanks!

@madjesc @bilelmoussaoui
Update .github/workflows/build.yml
6cc3574 
Co-authored-by: Bilal Elmoussaoui <[email protected]>
@Alecaddd
Copy link
Member

The build is busted.

@Alecaddd
Copy link
Member

@Mkefs you wrote Handle screenshot exception in the list of things to do.
What did you mean by that? Is that still an actionable item you want to tackle?

@madjesc
Copy link
Author

madjesc commented Jul 16, 2021 •
edited
Loading

@Mkefs you wrote Handle screenshot exception in the list of things to do.
What did you mean by that? Is that still an actionable item you want to tackle?

libportal pick_color_finish throws a GLib.Error if something went wrong. In ColorPicker.vala I don't handle this exception thus it does nothing when something wrong in libportal happens. Although it's harmless it should popup something to the user saying that something went wrong, or just log it to the console

// Libportal color picking finish 
// try {
GLib.Variant v = portal.pick_color_finish (result);
// } catch (GLib.Error e) { ... /* <-- Do something */ }

@Alecaddd
Copy link
Member

I can deal with that, thanks.
Another question. Is libportal available anywhere as an installable package with apt?
I know it's kind of a silly question as we're fully transitioning to a full faltpak based build, but for those who locally build this via meson I'd like to avoid forcing them to manually install libportal.

@madjesc
Copy link
Author

madjesc commented Jul 16, 2021

I can deal with that, thanks.
Another question. Is libportal available anywhere as an installable package with apt?
I know it's kind of a silly question as we're fully transitioning to a full faltpak based build, but for those who locally build this via meson I'd like to avoid forcing them to manually install libportal.

Via apt in debian no, in other distros like Void Linux, Arch and I think Ubuntu too, yes.
In debian it's in the unstable branch.

Alecaddd
Alecaddd requested changes Jul 16, 2021
View reviewed changes
Copy link
Member

@Alecaddd Alecaddd left a comment

Choose a reason for hiding this comment

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

This works but it removes the fundamental feature that makes the color picker tool easy to use and superior to the default color picker. The zoom (magnifying glass) effect.
That is vital and we need to keep it.

@Alecaddd Alecaddd marked this pull request as draft July 16, 2021 04:05
@madjesc
Copy link
Author

madjesc commented Jul 16, 2021

Ok, I will try to do it using libportal

@mbfraga
Copy link
Collaborator

mbfraga commented Jul 29, 2021

Awesome stuff! I use wayland on my dev machine that I use for Akira...so this is quite nice :P Will review.

@mbfraga
Copy link
Collaborator

mbfraga commented Jul 31, 2021

Works on wayland (what I use now). So it definitely gets my upvote.

@Alecaddd Alecaddd deleted the branch akiraux:master September 15, 2024 00:34
@Alecaddd Alecaddd closed this Sep 15, 2024
@AshkanArabim AshkanArabim mentioned this pull request Oct 4, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bilelmoussaoui bilelmoussaoui bilelmoussaoui left review comments

@Alecaddd Alecaddd Alecaddd requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Color picker not working under Wayland
5 participants
@madjesc @Alecaddd @mbfraga @bilelmoussaoui @Sav22999