[Draft] Multiple top-level windows (and some questions) #75
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Dretch
commented
Jan 26, 2020
| @@ -76,6 +76,7 @@ patchInContainer (StateTreeContainer top children) container os' ns' = do | |||
| -- widget in the corresponding place, we need to replace the GTK widget with | |||
| -- one created from the declarative widget. | |||
| (i, Just oldChildState, Nothing, Just new) -> do | |||
| error "this shouldn't happen!" | |||
There was a problem hiding this comment.
I would like to call destroy here with oldChildState, but we can't do that because destroy also requires the old declarative widget - which we don't have.
Does this situation ever actually happen? It seems to me that if oldChildState exists then we should also have the old declarative widget (since that would have been used to create oldChildState).
…tDestroy.There is a stumbling block here the container Patching code, where it seems sometimes the old declarative widget is not known, even though we have the state...
Reject bad states by erroring, instead of attempting to handle them.
|
I am closing this in order to focus my energies on #76 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a draft of multiple top-level windows (requested in issue #45).
The windows are created outside of the regular tree of Gtk widgets by wrapping a widget with a special "window host" widget that maintains a new top-level window in addition to the original widget. The handle to the window widget is put into the custom state field of the
StateTreeNode(the original custom state of the widget is also kept).This seems to work okay, although I am not sure the design is that elegant. Some things are worth mentioning in particular:
In order to remove the windows when the wrapped widget is removed, I added a
destroymethod to thePatchableclass. An alternative option here would be to attach a "destroy" signal handler to the wrapped widget (that then destroys the window), but I thought keeping the control withingi-gtk-declarative(rather than involving Gtk event handlers) would be more predictable. Does this design make sense?There is some code in
Patch.hsthat presents a problem for thedestroymethod design - I'll add an in-line comment to elaborate on this.I vaguely wonder if the "extra state in the widget tree" idea could be generalised to something more useful. There could be "custom attributes" that would allow hanging bits of state (with their own patching behaviour) from widgets in the tree. This could be used to easily implement new top-level windows, but also other patching behaviour that could call arbitrary methods on widgets. This would provide something similar to custom widgets, but less flexible and more re-usable - since a
CustomAttribute Button Eventcould be attached to anyButton, along with any combination of other custom or regular attributes. Does this make any sense?Cheers!