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

Migration of button-pressed and button-released to new GtkGesture API. #16919

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Christian-Kr
Copy link

This pr is connected to #15923, which has been closed and reopened here on a new branch.

Hello to everyone,

this pr will migrate the first button-pressed and button-release to the new GtkGesture API. Please test if everything is working as before. I do not find any problem but I also might have missed something.

I will wait with all further migration to this event context, until this pr has been reviewed. This is because I want to be sure, that the way I implemented it is your preferred way and I don't need to do things twice. :-)

Just to let all of you know: I am coming from the cpp perspective. This might break some things in code structure in the way I prefer to implement it.

Looking forward to your reply.
Thanks in advance

Greetings

Copy link
Collaborator

@ralfbrown ralfbrown left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@Christian-Kr
Copy link
Author

It would be awesome if anyone can set the label to gtk4, cause I can't. Thanks in advance.

Greetings

@elstoc elstoc added the gtk4 label Jun 2, 2024
@Christian-Kr
Copy link
Author

I got a short question: As I wrote in the introduction of this pr, I got a lot of additional changes if the first change here is fine for everyone. Just because of the implementation style. Now should I open a separate pr when this one has been accepted or should I keep on pushing to this pr branch with all further changes and tell here when I am ready?

I got a lot of changes in the pipeline with the new GtkGesture API. :-)

Thanks in advance.

@TurboGit
Copy link
Member

TurboGit commented Jun 3, 2024

Now should I open a separate pr when this one has been accepted or should I keep on pushing to this pr branch with all further changes and tell here when I am ready?

It all depends, I do prefer separate PR for separate issues and it gets easier to review. But seems like the other changes are GtkGesture related I would push on this same PR on separate commits to ease the incremental review. I would then merge this when accepted by squashing together all changes in a given PR.

Hopefully we would be able to start reviewing and integrating those changes after 4.8 is released.

Hope this plan is ok to you.

@TurboGit TurboGit added this to the 5.0 milestone Jun 3, 2024
@ralfbrown ralfbrown mentioned this pull request Jun 21, 2024
@TurboGit
Copy link
Member

After more discussion with the dev having worked on the Gtk+ code base it seems preferable to have a dedicated branch for all the Gtk4 code migration. So I have changed my mind and I'd like to go this path. A single PR with all commits for the Gtk4 which would be merged when ready. The rational is that the step by step migration could introduce some issues (even minor) and could cause multiple releases to have unstable (or annoying GUI glitch).

Hope you'll agree with the approach.

If so, we will try hard (I promise) to not change too much the Gtk code while it is being migrated to Gtk4 to avoid introducing more work and merge conflicts.

@Christian-Kr
Copy link
Author

Hello to everyone,

no problem for me. As you might have recognized, I got less time the last few month :-(. Hopefully I got more time for migration in the end of the year. Sry for the delay. Feel free to take my code changes till here and bring them into the process again till I got more time.

Good luck for everyone and thanks for all your work.
Greetings

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.

4 participants