Closed clstaudt closed 1 year ago
I tried the following implementation:
preferences/view.py
)PreferencesIntent.reset_app
Page.window_close
PreferencesIntent
needs the Page
every time it is instantiated (?)@vlad-ed-git How did you solve the problem of passing the Page
throughout the app's design? How should it be handled hereร
I don't pass the page. Just callbacks passed on to screens that are handled by app.
@vlad-ed-git Then how would you call Page.window_close
on the click of a button on the Preferences screen?
I would pass a close_window callback to Preferences screen , and have an app method to close the window.
@vlad-ed-git And how would the app's method get to the many places where a PreferencesIntent
is instantiated?
I pass these callbacks to the views (see routing section in the app.py file), not to the intent. The view can call close window , it does not need an intent for that.
@vlad-ed-git So like this?
app.close
via a callbackapp.clear_database
via a callbackSeems like either
app.clear_database
seems quite intuitiveIf clearing is done by calling clear database, then both app.clear_database and app.close are done from the view. Just merge them into one callback method.
something like on_reset_app . This can then clear database, and close the window.
If clearing is done by calling clear database, then both app.clear_database and app.close are done from the view. Just merge them into one callback method.
Okay, but clearing the DB is intuitively a data source concern. Resetting the app I would consider an intent, that may need error handling. Isn't this inconsistent with the MVI pattern? ๐ค
Yes it is, but having to access the app to clear the database is the source of the inconsistency.
Creating the database, and the models, and clearing them shouldn't be in the app's logic. See what I am doing with the other kind of storage we have (the one we use for preferences) ClientStorageImpl. It is part of the core. Flet's implementation of the client storage requires the page. So I am forced to instantiate the storage from the app and pass the page to it, and then provide access to this client storage object as part of TuttleParams to every screen. But even here, I do not implement the storage in the app.
@clstaudt please checkout and review this branch dev-db-refactor
@clstaudt the way dev-db-refactor
handles the database instantiation, offers more consistency. But it's incomplete, as we now have two issues, a good example of both is in the Auth.ProfileScreen view:
The client storage is requested by the intent from the views (which they receive as part of view params). Ideally, the intent should have access to the client storage themselves without needing the view, unfortunately, we cannot achieve this because flet's client storage requires a page. Unless of course we implement client storage in another way, without using flet's way.
more important for us the views have direct access to the database implementation also (the profile screen for instance has a call to self.database_storage.install_demo_data()
). I will fix this. But let's proceed with this reset button feature, then I shall refactor it.
I will do a quick & dirty implementation of the reset button then, because that's a feature needed for testing. A Pull Request is linked. ๐๐พ
@clstaudt Yes. Do that, then I shall refactor. Let me know which branch you are working from. If you have not started, you can do it from dev-db-refactor
(it is up to date with dev
).
I have started, the draft PR is attached to the ticket (by the way, we should do that always). Will check your branch later @vlad-ed-git
@vlad-ed-git I feel that the reset button is working fine for now with the quick&dirty implementation. Would you still like to make changes? If not please close the ticket.
@clstaudt I added a confirmation dialog, as this is a dangerous
action. I have no further changes so I will close this.
A reset button should allow the user to revert the app to its default state. The app should then quit.