getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.27k stars 167 forks source link

Deleting a file triggers unsaved changes bar only when file from same page is deleted. #2296

Closed texnixe closed 3 years ago

texnixe commented 4 years ago

Describe the bug
Don't know if this is a bug, but certainly inconsistent behavior. If you have a files field that allows you to select files from the same page, and delete a file selected in that field, the file is removed from list of selected files and the unsaved changes bar pops up, so you can save the page and the changes in the select field will be reflected in the content file.

However, when the field allows you to select files from other pages (e.g. all siblings) and you remove a file from a page other then the current page, the file is still removed from the list of selected files but the unsaved changes bar does not pop up, so that you are bereaved of the chance to save these changes.

To Reproduce
Steps to reproduce the behavior:

  1. In the Starterkit, add a query to the cover field in album.yml: query: page.siblings.images
  2. In the Panel, in an Album page, select an image from another than the current page and save.
  3. Click on the selected image, and in the file view, delete the image.
  4. Go back to thee original page. See how the image has disappeared from the list but no unsaved changes bar is shown.

Expected behavior
It would be useful if Kirby checks if there is a difference between what is stored in the page and the store and then show the unsaved changes bar.

Feel free to move this to the ideas repo if you don't think it's a bug.

Kirby Version
Tested with 3.3.0

distantnative commented 4 years ago

Documenting thoughts/progress:

It's hard to the Panel to show anything, since the backend already filters out the invalid value (since it cannot find the file anymore). So for the Panel it seems like content file and Panel store are the same.

Same behavior for pages and users btw.

I don't have a good idea yet what to do with this...

distantnative commented 3 years ago

I think this won't be solvable to be honest.

lukasbestle commented 3 years ago

I wonder why it works if the file is from the current page, but not if a custom query is used. @distantnative Have you found out why this is the case?

distantnative commented 3 years ago

Because then the files field can listen to the emitted event from the section and react. But if corresponding field and section are not on the same view, they have no chance to catch such an event

lukasbestle commented 3 years ago

Ah, right. But couldn't we fix this in general with the new Fiber setup? It knows that the file doesn't exist anymore on the backend and it also knows that the file is currently still referenced by the files field. Only problem: The unsaved changes bar is a frontend feature at the moment. But once we handle unsaved changes on the backend as well, it should be doable, no?

distantnative commented 3 years ago

Hard to say without a clear concept for the unsaved changes stored on the server, e.g. if it has performance implications adding additional checks. We'll have to see then.

With the unsaved changes as we have them, I don't think there's a reasonable way to solve this with Fiber either.

lukasbestle commented 3 years ago

OK, then I think we should postpone this issue until we start working on the "unsaved changes on the backend" feature.

bastianallgeier commented 3 years ago

I will close this as a weird side-effect of our complex content store implementation.