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

Experimental composable widgets for styling #14

Closed
wants to merge 2 commits into from

Conversation

DJMcNab
Copy link
Member

@DJMcNab DJMcNab commented Nov 29, 2022

Inspired by https://tonsky.me/blog/clojure-ui/#tweak-and-reuse and Tailwind CSS, this demonstrates how our current widget model lends itself to view level styling.

For reference, the user-facing part of this is:

fn app_logic(data: &mut f64) -> impl View<f64> {
    background(
        Color::RED,
        padding(
            *data,
            button(format!("Padding {data}px"), |data| *data += 1.),
        ),
    )
}

Which creates the following application.

Initial state:
image

After 37 clicks:
image

Beautiful, I know.

PaddingView::new(width, view)
}

pub struct PaddingView<V> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Here's an implicit decision about naming convention, that I really like;

It would be grammatically more obvious to name this PaddedView.

But I think this way it's way nicer, and we should adopt this as a naming convention.

E.g. BorderView instead of BorderedView etc.

This way it's a little easier to navigate the stuff xilem has to offer, even if your english is good. But more than anything, we make the library a little more accessible to non native speakers, who struggle with the language barrier

@nilsmartel
Copy link
Contributor

If I am not mistaken, using piets clipping abilities, it would be possible to adopt a "BorderView" as well, rounding the borders of an element, right?

I'm just curious what this system is capable of doing :)

@DJMcNab
Copy link
Member Author

DJMcNab commented Dec 3, 2022

Borders is an open question for me

From a conceptual viewpoint it's easy; just draw a line along the parent clipping path, or a rectangle if the parent isn't a clip.

In terms of actual implementation, I'm less sure how that fits into our painting model at the moment.

@Speykious
Copy link

Hi, I'm currently looking at Xilem because I am fascinated by the project. I found this pull request and decided to experiment with it, so I made the padding view have individual sizes for all 4 sides:
image
image

(not sure if you want me to add that in, just thought it would be cool to show ^^)
Since we're talking about how we could implement a BorderView, @DJMcNab what makes you unsure about how it fits into the painting model? I know the project relies on piet-gpu for drawing but I'm not too familiar with its current state, or even with its API at the moment...

@rosefromthedead
Copy link
Contributor

I played around with this a little, and ended up implementing a border widget too. I also found that for simple wrapper widgets like these there isn't really a need for a Pod - especially for the background widget, all its implementation needs to do is paint a rectangle before forwarding all of the calls to the inner widget. I'm interested whether people think this Pod-free way of creating wrapper widgets is a good idea, since it probably eliminates a little runtime checking and allocating?

My code is here.

@DJMcNab
Copy link
Member Author

DJMcNab commented Jan 30, 2023

I wanted to avoid the Pod, but I found issues with widgets inspecting PaintCx::size, for example. It seemed to me that the traits are designed such that Widgets were intended to only exist with Pods.

PaintCx is not externally constructible, so "spoofing" that isn't intended. And button reads cx.size() to get the area to draw.

That is to say, I would like to avoid the Pod. But working within the constraint of changing nothing else, it doesn't work.

@rosefromthedead
Copy link
Contributor

Oh yeah, size is the one thing I hadn't figured out yet, and I assumed it would be fairly straightforward like the mouse events were... oops. For background at least, it feels like that can still be kept podless.

About the border thing, I think I accidentally avoided the problem you mentioned (how to know the parent clipping path) by just making the clip and the border be part of the same widget.. maybe that's the easy way out. I can't think of a reason why a user might need them to be different widgets, and worst case the path could just be specified twice.

@DJMcNab
Copy link
Member Author

DJMcNab commented Jan 30, 2023

Yeah, joining clip and border seems reasonable. One could imagine wanting to have different kinds of border (e.g. an image based border), but that could probably be its own copy-pasted widget.

@DJMcNab
Copy link
Member Author

DJMcNab commented Jun 13, 2023

So, the Xilem Vector Graphics talk has clarified this for me slightly.

Broadly, I still think the styled views are compelling, but the implementation methods using a unique widget for each is not needed

@DJMcNab DJMcNab closed this Feb 8, 2024
@DJMcNab DJMcNab deleted the experimental-compose branch May 15, 2024 21:30
github-merge-queue bot pushed a commit that referenced this pull request Nov 11, 2024
New tracy image:


![image](https://github.com/user-attachments/assets/94e54c89-8159-4dd4-a521-4a7122f64375)

New log tracing file:
<details><summary>An overview of the new logs</summary>
<p>

```
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}: masonry::passes::update: RootWidget received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}: masonry::passes::update: Flex received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}: masonry::passes::update: Flex received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}:SizedBox{id=#7}: masonry::passes::update: SizedBox received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}:SizedBox{id=#7}:Flex{id=#6}: masonry::passes::update: Flex received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}:SizedBox{id=#7}:Flex{id=#6}:Flex{id=#3}: masonry::passes::update: Flex received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}:SizedBox{id=#7}:Flex{id=#6}:Flex{id=#3}:Prose{id=#1}: masonry::passes::update: Prose received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}:SizedBox{id=#7}:Flex{id=#6}:Flex{id=#3}:Label{id=#2}: masonry::passes::update: Label received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}:SizedBox{id=#7}:Flex{id=#6}:Flex{id=#5}: masonry::passes::update: Flex received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#8}:SizedBox{id=#7}:Flex{id=#6}:Flex{id=#5}:VariableLabel{id=#4}: masonry::passes::update: VariableLabel received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}: masonry::passes::update: Flex received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#10}: masonry::passes::update: Button received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#10}:Label{id=#9}: masonry::passes::update: Label received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#12}: masonry::passes::update: Button received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#12}:Label{id=#11}: masonry::passes::update: Label received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#14}: masonry::passes::update: Button received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#14}:Label{id=#13}: masonry::passes::update: Label received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#16}: masonry::passes::update: Button received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Flex{id=#17}:Button{id=#16}:Label{id=#15}: masonry::passes::update: Label received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}: masonry::passes::update: Portal received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}: masonry::passes::update: Flex received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#24}: masonry::passes::update: SizedBox received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#24}:Flex{id=#23}: masonry::passes::update: Flex received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#24}:Flex{id=#23}:Flex{id=#20}: masonry::passes::update: Flex received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#24}:Flex{id=#23}:Flex{id=#20}:Prose{id=#18}: masonry::passes::update: Prose received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#24}:Flex{id=#23}:Flex{id=#20}:Label{id=#19}: masonry::passes::update: Label received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#24}:Flex{id=#23}:Flex{id=#22}: masonry::passes::update: Flex received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#24}:Flex{id=#23}:Flex{id=#22}:VariableLabel{id=#21}: masonry::passes::update: VariableLabel received Update::WidgetAdded
14:37:40.365Z TRACE update_new_widgets:RootWidget{id=#165}:Flex{id=#164}:Portal{id=#163}:Flex{id=#160}:SizedBox{id=#31}: masonry::passes::update: SizedBox received Update::WidgetAdded
```

</p>
</details> 

This was originally an experiment into caching spans, but I determined
that was non-viable due to the pass names.
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 this pull request may close these issues.

4 participants