docs: todo example to use a class for state #667
Conversation
in place of dataclass with Ref() and fields
| @@ -141,7 +97,7 @@ def create_new_item(*ignore_args): | |||
| ] | |||
|
|
|||
|
|
|||
| # We store out reactive state, and our logic in a class for organization | |||
| # We store our reactive state, and our logic in a class for organization | |||
| # purposes, but this is not required. | |||
| # Note that all the above components do not refer to this class, but only | |||
There was a problem hiding this comment.
these in-line comments may need updating?
| @@ -141,7 +96,7 @@ def create_new_item(*ignore_args): | |||
| ] | |||
There was a problem hiding this comment.
A comment here would be helpful explaining why this is globally declared and not created as a class attribute inside State or inside State.init().
| """Takes a reactive todo item and allows editing it. Will not modify the original item until 'save' is clicked.""" | ||
| copy = solara.use_reactive(todo_item.value) | ||
| copy = TodoItem(todo_item.text, todo_item.done) |
There was a problem hiding this comment.
It appears to me that this does not create a "deep" copy, but uses the same reactive objects in the todo_item, so that any change to the "copy" will actually modify also the todo_item.
| # Note that all the above components do not refer to this class, but only | ||
| # to do the Todo items. |
There was a problem hiding this comment.
This could use further justification and explanation.
| @@ -151,25 +106,23 @@ def create_new_item(*ignore_args): | |||
|
|
|||
There was a problem hiding this comment.
If this is an example of how to manage state, it would be even more valuable to show how the todos can be reused. Now the Page component is effectively a Todo component, but it's not reusable as State can only manage a single list of Todos.
| @@ -183,14 +136,16 @@ def TodoStatus(): | |||
| solara.Success("All done, awesome!", dense=True) | |||
|
|
|||
|
|
|||
| state = State(initial_items) | |||
|
|
|||
|
|
|||
| @solara.component | |||
| def Page(): | |||
| with solara.Card("Todo list", style="min-width: 500px"): | |||
There was a problem hiding this comment.
Please consider doing a separate Todo component card and show how it can be reused. Again, this may be another example (a dashboard of TODO lists for different people/tasks where you can add or remove entire TODOs?)
| @@ -183,14 +136,16 @@ def TodoStatus(): | |||
| solara.Success("All done, awesome!", dense=True) | |||
|
|
|||
|
|
|||
| state = State(initial_items) | |||
There was a problem hiding this comment.
It would be nice with a comment explaining how the state object can still work for multiple clients (virtual kernels), despite it not being declared a solara.reactive.
maartenbreddels
force-pushed
the
master
branch
2 times, most recently
from
86646f5
to
df2fd66
Compare
This PR updates the todo example to place all state in a (re-usable)
TodoItemclass, instead of adataclass. This allows avoiding the need forRefand.fields.If there is no other example for
Refandfieldsin the docs, then maybe this shouldn't be merged, but at least shows the difference between the two implementations.