Implement manual autosave control #624
Conversation
src/Client/LocalStorage/Darkmode.fs
Outdated
There was a problem hiding this comment.
don't change stuff you are not touching ... !! 😄
There was a problem hiding this comment.
especially if it is a code style flavor based on personal preference. After the next release i will add fantomas support to avoid this issue for now and forever
There was a problem hiding this comment.
dont touch stuff you are not working on
src/Client/Messages.fs
Outdated
There was a problem hiding this comment.
dont touch stuff you are not working on
src/Client/Model.fs
Outdated
| SwateDefaultSearch : bool | ||
| TIBSearchCatalogues : Set<string> | ||
| [<AutoOpen>] | ||
| module PersistentStorageState = |
There was a problem hiding this comment.
no! do not abbreviate from existing patterns
src/Client/Model.fs
Outdated
| } | ||
| open Browser | ||
|
|
||
| let [<Literal>] AutosaveConfig_Key = "AutosaveConfig" |
There was a problem hiding this comment.
Move this to "LocalStorage" folder with its own file
There was a problem hiding this comment.
dont do code style changes on files you are not working on
There was a problem hiding this comment.
dont do code style changes on files you are not working on
| @@ -81,13 +82,13 @@ type Settings = | |||
| ] | |||
| ) | |||
|
|
|||
| static member ActivityLog(model) = | |||
| static member ActivityLog (model, dispatch) = | |||
There was a problem hiding this comment.
as mentioned above, move this out of activity log
src/Client/States/LocalHistory.fs
Outdated
There was a problem hiding this comment.
dont do code style changes on files you are not working on
src/Client/Update/InterfaceUpdate.fs
Outdated
There was a problem hiding this comment.
dont do code style changes on files you are not working on
|
|
| open Messages | ||
|
|
||
| [<AutoOpen>] | ||
| module rec PersistentStorage = |
There was a problem hiding this comment.
Why have you added "module rec PersistenStorage"?
As far as i can see it it is not required and should not exists in ActivityLogView.
What you could do: Move ActivityLog as a file to Pages/Settings. Since the rework some months ago, activity log is no longer their own page, but was added to settings
| prop.children [ | ||
| Html.p [ | ||
| prop.className "select-none text-xl py-2" | ||
| prop.text "Autsave" |
| @@ -0,0 +1,20 @@ | |||
|
|
|||
| module AutosaveConfig | |||
There was a problem hiding this comment.
module LocalStorage.AutosaveConfig ! (Reuse existing patterns)
| prop.text "Autsave" | ||
| ] | ||
| Daisy.toggle [ | ||
| let autosaveConfig = getAutosaveConfiguration() |
There was a problem hiding this comment.
i think this is bad practise. imo better use a react state and fill it with React.useEffectOnce
There was a problem hiding this comment.
no wait. this is simply wrong. "autosaveConfig" refers to value stored in LocalStorage. But value in LocalStorage should be read into PersistentStorage model on init. So you do not need it here
|
|
||
| prop.onChange (fun (b: bool) -> | ||
| PersistentStorage.UpdateAutosave (model.PersistentStorageState.Autosave) |> PersistentStorageMsg |> dispatch | ||
| setAutosaveConfiguration(b) |
There was a problem hiding this comment.
This setAutosaveConfiguration should be done inside of update function for "UpdateAutosave" PLUS you are not using b to update model but the value already in model. So this should never change?
| Components.Forms.Generic.BoxedField("Appearance", | ||
| static member ToggleAutosaveConfig(model, dispatch) = | ||
| Html.label [ | ||
| prop.className "grid lg:col-span-2 grid-cols-subgrid cursor-pointer not-prose" |
| prop.onChange (fun (b: bool) -> | ||
| PersistentStorage.UpdateAutosave (model.PersistentStorageState.Autosave) |> PersistentStorageMsg |> dispatch | ||
| setAutosaveConfiguration(b) | ||
| if not b then LocalHistory.Model.ResetHistoryWebStorage() |
There was a problem hiding this comment.
also move into update for PersistentStorage.UpdateAutosave
Enable manual control of the autosave