Skip to content
This repository has been archived by the owner on Jun 8, 2021. It is now read-only.

subclass/widget: Implement default handling for parent events #921

Merged
merged 2 commits into from
Dec 7, 2019
Merged

subclass/widget: Implement default handling for parent events #921

merged 2 commits into from
Dec 7, 2019

Conversation

alatiera
Copy link
Contributor

@alatiera alatiera commented Dec 4, 2019

Close #916

.expect("No parent class impl for \"button_press_event\"");
let ev_glib = glib::translate::mut_override(event.to_glib_none().0);
Inhibit(from_glib(f(widget.to_glib_none().0, ev_glib)))
if let Some(f) = (*parent_class).button_press_event {
Copy link
Member

Choose a reason for hiding this comment

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

Can this part be macro?
call_parent!(button_press_event, f(widget.to_glib_none().0, ev_glib))

Copy link
Member

Choose a reason for hiding this comment

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

No because it looks different depending on what the parent class expects by default.

Copy link
Member

Choose a reason for hiding this comment

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

Ok.

@sdroege
Copy link
Member

sdroege commented Dec 4, 2019

  • parent_child_notify() should do nothing instead of panicking
  • parent_compute_expand() is difficult. We need to replicate the behaviour the base class would do if it's not set here... basically we would need to set the two bools to the return values of gtk_widget_get_hexpand() and gtk_widget_get_vexpand().
  • Application, container, dialog, window need the same treatment (I think all of them are cases of "do-nothing-if-not-set")

@sdroege
Copy link
Member

sdroege commented Dec 6, 2019

Looks good, I think. Only container and widget's compute_expand() missing now :)

@alatiera
Copy link
Contributor Author

alatiera commented Dec 6, 2019

Added compute expand now, though given how simple it looks I am not sure if that's what you had in mind?

@sdroege
Copy link
Member

sdroege commented Dec 6, 2019

Added compute expand now, though given how simple it looks I am not sure if that's what you had in mind?

Yes exactly that. Then only the GtkContainer virtual methods are missing and then we're good to go here :)

@alatiera
Copy link
Contributor Author

alatiera commented Dec 6, 2019

oh whoops, forgot aobut container

@sdroege
Copy link
Member

sdroege commented Dec 6, 2019

👍

src/subclass/container.rs Outdated Show resolved Hide resolved
@alatiera
Copy link
Contributor Author

alatiera commented Dec 7, 2019

Should be good to go!

@sdroege
Copy link
Member

sdroege commented Dec 7, 2019

Should be good to go!

I agree :) Also your examples PR should be good to merge then?

@alatiera
Copy link
Contributor Author

alatiera commented Dec 7, 2019

Okay, that's the last of the panics I am hitting it seems \o/

@sdroege
Copy link
Member

sdroege commented Dec 7, 2019

Okay, that's the last of the panics I am hitting it seems \o/

🙌 Let's get this in then once the CI is green

@GuillaumeGomez
Copy link
Member

Thanks!

@GuillaumeGomez GuillaumeGomez merged commit 2efce6e into gtk-rs:master Dec 7, 2019
@alatiera alatiera deleted the alatiera/widget-parent-events branch December 7, 2019 17:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Application subclasss needs to handle parent delete_event
4 participants