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

template_child: Implement PropertyGet for TemplateChild #1886

Merged

Conversation

felinira
Copy link
Contributor

This circumvents the drop issue for now by just not allowing property setters at all. Then we can have the field be transparent again. :)

As we don't have public setters on TemplateChild it creates an inconsistency where setting via Property would work, but setting the field directly wouldn't.

I think we should either:

  • Generally allow setting the value of TemplateChild via a public set method
  • Not allow it at all (which this opts to do)

This is technically breaking, because it unimplements HasParamSpec, although I can't really see any reason to rely on this in application code. 🤷‍♀️

Copy link
Member

@bilelmoussaoui bilelmoussaoui left a comment

Choose a reason for hiding this comment

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

As I mentioned on the Matrix channel, I don't use this feature on any of the apps I maintain, nor have an idea about the expectations from GTK side on whether you can modify the pointer that gets initialized with gtk_init_template.

So I am fine with merging this change. It would be nice to confirm if template children widgets as properties can not be writable before doing so though.

@sdroege
Copy link
Member

sdroege commented Oct 25, 2024

Your call but I think this makes sense. You could implement PropertySet if you want writable properties at a later time

@bilelmoussaoui
Copy link
Member

Let us go with this, if someone complains with lack of PropertySet, then we can figure out how/why later

@bilelmoussaoui bilelmoussaoui merged commit 9285c1b into gtk-rs:main Oct 25, 2024
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants