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

Patchable for Box ignores BoxChildProperties? #51

Closed
niteria opened this issue Jul 9, 2019 · 2 comments · Fixed by #71
Closed

Patchable for Box ignores BoxChildProperties? #51

niteria opened this issue Jul 9, 2019 · 2 comments · Fixed by #71

Comments

@niteria
Copy link

niteria commented Jul 9, 2019

I was trying to understand how the pieces fit together and looking at the implementation of Patchable for Box got me wondering why BoxChildProperties isn't taken into account.

I don't have a real world example that demonstrates it should be, but I can come up with something contrived that demonstrates the need. Say the padding property is derived from the current application state. IIUC (and a simple program seems to confirm this) an empty patch will be computed and nothing will update.

Am I misunderstanding something?

(Btw, thank you for the library, the library looks awesome, I really want to try building UIs in this style)

@owickstrom
Copy link
Owner

Oh, I think you are right. https://github.com/owickstrom/gi-gtk-declarative/blob/master/gi-gtk-declarative/src/GI/Gtk/Declarative/Container/Box.hs#L54 is missing updates for the properties. I'm not sure we can update the box packing properties without removing the children and readding them, though.

BTW, I've been working on a property-based testing setup which actually runs GTK so that we can test this library properly. I'll try to reproduce this particular problem in such a test.

@owickstrom
Copy link
Owner

If you want to try fixing the issue in the meantime, that would be very welcome. Thanks for the kind words!

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 a pull request may close this issue.

2 participants