search

linebender / xilem

An experimental Rust native UI framework
Apache License 2.0
3.55k stars 111 forks source link

xilem_core: Provide a way to add `View` implementations for external types #402

Closed Philipp-M closed 3 months ago

Philipp-M commented 3 months ago

Since we're suffering from the orphan rule with the new xilem_core implementation, we cannot use View in downstream crates for external types such as a WidgetView implementation for &'static str in Xilem.

This PR adds an OrphanView trait which the Context of the View has to implement for the type which has an implementation in xilem_core for this. The View impl for these types is basically a proxy for OrphanView (which in turn has the same method signature as View), so downstream crates can achieve the same as with the View.

Philipp-M commented 3 months ago

It's not clear why you've added a std feature for this; none of the things behind it actually need it.

I guess I wasn't thorough enough and thought, these require std, even better if that's not necessary, I've removed the std feature.

I'm also not convinced it's meaningful to have a View implementation for numbers. It's cute, but it's a bit weird imo.

xilem_web actually already had support for numbers as Text node, apart from syntax-sugar and being slightly more optimized than using something like format!() or num.to_string(), I do think it can be useful in a more mathematical focused impl of xilem_core. I haven't looked into how viable xilem_core could be e.g. for that animatables experiment that I did in trui, but that's e.g. a use-case (i.e. animatable properties).

shows two different ways (unfortunately they aren't possible for the same type at the same time)

So at this point I'm leaning towards the more traditional approach with providing an actual View impl (in the form of OrphanView in the context of this PR), so I'm thinking about removing AsOrphanView in favor of OrphanView as it allows more control with just little bit more syntactical overhead. Maybe we can get similar behavior like as_view() somehow else in a more general way (without extra marker in View, because I don't think that gives good DX TBH).

Philipp-M commented 3 months ago

I've removed the AsOrphanView, cleaned up a little, and added a "test" (more to show how it's used, as the implementation is trivial). When there's no objection that would be ready to merge/review now. The kurbo feature is used in the xilem_web rewrite already.

Philipp-M commented 3 months ago

"AsView" style is that I think it's important that these views are only syntax sugar for an existing view

Yep that's why I originally wanted to have both of these (and the full View equivalent to avoid allocs for more optimized views), but these unfortunately intersect... I really hope we can find ways in general for something like this, because I think it's quite useful to avoid a full View impl. especially for end users not being familiar with all the specifics of the View trait.

Thanks for the review and the initial impl in https://github.com/DJMcNab/xilem/pull/4 that inspired this.