Make xilem::Pod more flexible

[DJMcNab]

Previously, the use of WidgetPod made it impossible to create "wrapper" widgets for certain properties. This new design "decomposes" that WidgetPod for use in Xilem.

Open question: Is this new type more widely useful to other users of Masonry? Xilem would need a wrapper
Open question: How will having multiple event handlers for a single widget work? I imagine they would all use the same view path, and pass on the event if it's not relevant.

Follow-up work:

• Splitting Button so that the handlers are wrappers
• Maybe: Splitting TextArea so that the handlers are wrappers (ooh, we can probably properly support different binding models!)
• Removing the unnecessary and bloated transform field from each Widget.

[DJMcNab]

This is the main API change in the new version

[Philipp-M]

Removing the unnecessary and bloated transform field from each Widget.

There's a reason why I've done it like this, but that's definitely worth a discussion itself:

My goal of the API is/was, to allow setting all kinds of attributes after having modified the transform, AFAICS, this is not easily possible when composing views without some type-fu, and also having the option to return the opaque widget type (fn my_sized_box() -> SizedBox<State>), which I think leads to undesired behavior on the user-API-side.

To achieve this with composition, I think a similar pattern as in xilem_web with extension traits (DOM interface traits) is necessary, which brings all kinds of complex edge cases to handle this robustly, depending on the overwrite/extension behavior, and whether the underlying view/widget can change, e.g. with OneOf. But that might be less of an issue in xilem native because the widget (e.g. OneOfWidget) doesn't change like in xilem_web, just the inner contained widget.

So the simple/straight-forward, but as noted bloated solution which is currently in main, is just adding an additional attribute to each widget view, which IMO was the trade-off I wanted to make to achieve that API goal (i.e. just implementing a minimal trait, and handling the diffing/setup in each widget view, which is obviously boilerplate and not optimal...).

[Philipp-M]

I'm not sure if this is really optimal, I think it should be possible to overwrite an already set transform.

[Philipp-M]

I think this breaks or leads do undesired behavior, when composing two Transformed views. E.g. when the inner changes, the outer not (as also noted here: #591 (comment)). As noted, also one of the reasons for the somewhat complex implementation of xilem_webs attribute overwrite behavior.

(Noted after writing this, that this may lead to a panic above, but then this still has this issue, if the underlying transform is Affine::IDENTITY)

[DJMcNab]

Yes... this is exactly the reason for the above panic.

then this still has this issue, if the underlying transform is Affine::IDENTITY

Yes - Pod::transform should be an Option, to make this robust.

[DJMcNab]

I can't see how requiring there to be 48 bytes of extra data (which will always be Affine::IDENTITY; the cases where this is set is a rounding error) on each view is a worthwhile tradeoff here, though. It is entirely possible to work around this at the moment, by just setting the properties you are interested in other than transform first. Like yes, it would be nice, but surely not nice enough to justify the solution we have currently?

I imagine that most property setting wrapper views would operate on WidgetView<Widget=SomeConcreteType>.

[jaredoconnell]

I'm not familiar with the design intricacies of the transform, so I don't have any comment on that, but there is a great need for widgets that support composition, so it's nice to see progress on that.

[Philipp-M]

I can't see how requiring there to be 48 bytes of extra data (which will always be Affine::IDENTITY; the cases where this is set is a rounding error) on each view is a worthwhile tradeoff here, though

Yeah, same is true for all kinds of other attributes (also specific to just the widget, when they're not used often) - the more the worse it will get, and composing types will be more reasonable. I'm just not sure where to draw the line, or whether/how much we should mix composition of (widget-specific) attributes with "big-struct" views. I expect these kinds of cases to be more often in the future...

As you mention that, this (extra bytes of data, stack size) was (one of) the reason(s) for the design of composing all kinds of attributes with types in xilem_web.

Like yes, it would be nice, but surely not nice enough to justify the solution we have currently?

I think it depends on what the focus of the API is, I've focused mostly on the end-user experience, so in that regard maybe yes? That said, I do think long-term we want to think more about composition, with increasing attribute-count, with a consistent API, ideally less complex than in xilem_web, but that needs careful design...

I imagine they would all use the same view path, and pass on the event if it's not relevant.

I think that's kind of related to the "composing attributes with types" topic, in xilem_web it's separate views and thus view id paths, if the handler should stay in the same struct (as it is now), I think it's as you say with the same view id path, maybe marked and routed with an Event type (using TypeId?).

[Philipp-M]

Code looks good.

I'd prefer though, having the transform change in its own PR, I think it's at least somewhat controversial (with pros/cons for either composition/big-struct), and while it certainly would reduce error-prone boilerplate (and memory on the stack), I think a regression in UX of the API of Xilem.