So I'm working on improving the focus management and I came across an idea. However, not one that I felt comfortable just dropping a PR for without opening it up to discussion first (since it could just be a horrendous idea). So here's the problem I found and a possible solution I came up with.
## Problem
A lot of internal stuff is exposed to the end user that either doesn't need to be or shouldn't be. At best it can be confusing or misleading to users, and at worst it can cause some users to use code in ways they shouldn't (i.e., unintentional hacks that are unstable and could break between versions).
For example, it seems wrong that a user can do this: `self.set_id(Index::default())`. And while this might not be that big of an issue ( "users should just *not* do that" ), what if we want to add other functionality onto widgets in the future? Maybe we add the ability to give widgets their own classes for theming. This might be confusing for the user who thinks that `self.add_class("foo")` should work on its own (but might not since the context also needs to be made aware of the change).
Additionally, `KayakContext`, has a lot of functionality that doesn't need to be exposed within the context of a widget, but needs to elsewhere (such as the `KayakContext::render` and `KayakContext::process_events` methods). Methods like these need to be public, but they're dangerous to use within a widget's render method.
So in summary the two main issues are:
1. Internal widget data is exposed to the user in the render function in a confusing manner
2. Error-prone `KayakContext` methods are also exposed to the user in the render function when they shouldn't be
## Solution
I think the widget system could be restructured in a way that creates a layer of abstraction between the widgets and their context.
Let's assume we have this widget:
```rust
#[widget]
fnMyWidget(name:String,fill:Option<Color>){
// ...
}
```
### Props Extraction
To get around the first issue of the widget having too much mutable access to its required widget data, we could separate the render function from the widget itself by placing the actual logic inside an associated method.
This would remove the access to a mutable `self`. But we still need access to the actual props. To solve this issue we can generate a new struct, `MyWidgetProps`, to contain the prop data. We can then pass this into the render function.
And now we have something that looks a bit more like this:
Now comes the second issue of direct access to `KayakContext`. I think a good way of solving this would be to generate an interface struct. This struct would act as a middleman between the render function and `KayakContext`, forwarding only select methods to the context.
There are two ways of going about this.
1. Create a local struct:
```rust
structMyWidgetContext<'a>{
context:&'amutKayakContext,
widget:&'amutMyWidget
}
```
2. Create a one-size-fits all struct
```rust
structMyWidgetContext<'a>{
context:&'amutKayakContext,
widget:&'amutdynWidget
}
```
Though option 1 results in more code being generated, I actually like it better since it allows for greater customization in cases where the widget is written out manually instead of via cookie-cutter macro.
Using this interface struct, we can then replace the direct `KayakContext` access:
```rust
implMyWidget{
fnrender(mutthis:MyWidgetContext){
// Generated logic
}
}
```
> We can of course call the parameter `context` or something. I'm just being cheeky by calling it `this` here ;)
And of course we'll need some code for it to be useable:
```rust
/// We could also create a trait for common methods, but this is probably fine
/// since we'll likely only ever use it internally within a particular widget
/// (in other words, not as a passable trait object)
impl<'a>MyWidgetContext<'a>{
pubfnprops(&self)->&MyWidgetProps{
&self.widget.props
}
pubfnset_styles(&mutself,styles:Option<Style>){
self.widget.styles=styles;
}
// ...
}
implMyWidget{
fnrender(mutthis:MyWidgetContext){
letMyWidgetProps{name,fill}=this.props();
// Generated logic
}
}
```
And again, the purpose here is to hide internals so the user isn't confused and doesn't code in a hacky way:
```rust
implMyWidget{
fnrender(mutthis:MyWidgetContext){
// It's now impossible to change a widget's ID internally
self.set_id(Index::default());
// You can no longer render-ception
context.render();
// This might seem like a hack to dispatch events but it's an error
// Now we can make this properly notify the context
this.add_class("foo");
}
}
```
### TL;DR
1. Generate `MyWidgetProps`
2. Move rendering to associated function
3. Create `MyWidgetContext` to interface between render function and `MyWidget` and `KayakContext`
4. ???
5. Profit
## Alternatives
### Faith
The biggest alternative, again, is just to trust that the developer isn't going to cause any issues and that we make it clear what code *could* cause issues. But I think we can still do something to help. And coming up with a solution also improves the overall experience and API.
### Data-less Widgets
One big alternative to a portion of this is to just not store data on the widget itself. All relevant data for a widget will be stored in the `KayakContext`. This forces all code that needs a bit of data about a widget to also need access to the current context.
This could make code outside the functional component much more cumbersome to use.
Additionally, we run into the issue of storing the widget's ID, since that data does need to exist on the widget regardless.
### Context Setters
One of the issues I briefly mentioned was about the idea that updating a widget's own data does not directly alert the context. This could be solved by just having users write the notification code:
```rust
#[widget]
fnFoo(context:&mutKayakContext){
self.add_class("bar");
context.notify(KayakNotify::ClassAdded);// or however it's implemented
}
```
From an API perspective, this isn't great. Using the Widget Context idea, we can move that required code into the interface method:
```rust
impl<'a>FooContext<'a>{
pubfnadd_class(&mutself,class:&str){
self.widget.add_class(class);
context.notify(KayakNotify::ClassAdded);
}
}
#[widget]
fnFoo(){
this.add_class("bar");
}
```
Also note that we can't do this:
```rust
#[widget]
fnFoo(context:&mutKayakContext){
context.add_class("bar");
}
```
Since this requires that `Foo` have a public method `add_class` for the context to set. And the alternatives for those issues go back to the ones listed above.
### Move Render Function into Widget Context
One potential alternative to what's mentioned above would be to move the associated render function from `MyWidget` into the `MyWidgetContext`. This would allow `self` to be used as normal and even give direct access to `context` and `widget` if needed, while still granting the separation that allows us to add custom logic.
This is all just me thinking of ways to improve on these issues. Are they worth the trouble? And would we stand to gain from implementing a solution like this? I think so, but I could be totally and completely wrong. There could also be other, better solutions than the one I came up with.
Anyways, just wanted to bring this up and see what people thought. So let me know!
## Addendum
### Tracking Changes
An additional feature this might allow is tracking changes that can't be tracked during the render phase:
```rust
#[widget]
fnMyWidget(disabled:bool){
this.set_focusable(disabled);
// ...
}
```
And in `MyWidgetContext`:
```rust
pubfnset_focusable(&mutself,focusable:bool){
self.widget.focusable=focusable;
// If we need to track this change, we could do something like:
self.edits.insert(WidgetEdit::Focus);
}
```
Then if we need to handle any changes we can just iterate through the set of changes and handle them individually.
