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

Reload recipes after rescan #223

Merged
merged 3 commits into from
May 17, 2020
Merged

Reload recipes after rescan #223

merged 3 commits into from
May 17, 2020

Conversation

sam-19
Copy link
Contributor

@sam-19 sam-19 commented May 16, 2020

Resolves #168 and #183 (merge my other pull request first)

This performs just a crude refresh of the router view. For a more sophisticated solution (like only reloading the recipes and not the whole view, which would avoid the view flashing empty for a split second) some more fine-grained checking would be needed.

@sam-19
Copy link
Contributor Author

sam-19 commented May 16, 2020

Actually, there is a potential problem with this solution. Depending on how the update interval is supposed to work, this could potentially reload a recipe that a user is editing, reverting all fields to their original values. So this will need some more thinking, if the update interval is meant to automatically reindex the library.

@sam-19
Copy link
Contributor Author

sam-19 commented May 16, 2020

In fact, now that I think about it, this opens a whole can of worms. When a user begins editing a recipe, does the app lock that particular folder? If not, some way to resolve version conflicts has to be set up. I will remove edit from the views that are automatically refreshed for now. To think about it, I will also remove basic recipe view; after all, if you have the recipe open when it is deleted, that gives you a last chance to save the data in case deleting the recipe was an accident.

@mrzapp mrzapp merged commit 747d5a0 into nextcloud:v0.7 May 17, 2020
@mrzapp
Copy link
Contributor

mrzapp commented May 17, 2020

@sam-19 folders aren't locked as far as the API goes, it just writes files with whatever content is submitted in the "edit" form.

I honestly think we don't have to auto refresh the main content view, just the sidebar. Let's not overcomplicate things :)

@sam-19
Copy link
Contributor Author

sam-19 commented May 17, 2020

Right now it only refreshes the view if the user is on the index page or search page (i.e. there are likely many results on the screen), which I think the original issue was about. I could try to bake in a check, so it would only refresh the view if it actually has changed. Another possibility would be to show a message like "Results in this view have changed" and offer a reload button for the user.

@mrzapp
Copy link
Contributor

mrzapp commented May 17, 2020

@sam-19 I think it's fine to reload them automatically, but just exclude the edit and recipe views. Maybe that's what you already did, and I'm just slow 😅

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.

2 participants