Fix deletion of multiple files should show one confirmation dialog #9573
Conversation
There was a problem hiding this comment.
Hi, thanks for your interest in JabRef development and your proposed changes. I took a quick look into it and am a bit concerned that the changes do not really work well as they are right now.
I believe the whole delete stuff should move to a new class extending SimpleCommand to make this undoable eventually. This command could then be called at any place and should not depend on JavaFX dialog buttons.
Thanks!
| public boolean delete(Optional<ButtonType> buttonType, ButtonType removeFromEntry, ButtonType deleteFromEntry) { | ||
| Optional<Path> file = getDir(); |
There was a problem hiding this comment.
Please avoid passing Optionals as arguments.
Why at all is a ButtonType object being used in the ViewModel?
This does not seem right at all. The ViewModel should not have to deal with the ui elements, this breaks the mvvm pattern. Please use primitives here.
| @@ -589,4 +579,9 @@ private Supplier<BibEntry> wrapImporterToSupplier(Importer importer, Path filePa | |||
| } | |||
| }; | |||
| } | |||
|
|
|||
| public Optional<Path> getDir() { | |||
There was a problem hiding this comment.
Please don't use abbreviations. Try to be more concise, what directory is being looked for?
|
Closing this PR due to inactivity 💤 |
Fixes #9473
Moved code of delete dialog from
LinkedFileViewModeltoLinkedFilesEditorViewModel.Made a separate method in 'LinkedFilesEditorViewModel' for deleting multiple files
deleteFiles.Made a 'getDir' method in
LinkedFileViewModelto get the path of file.CHANGELOG.mddescribed in a way that is understandable for the average user (if applicable)Deleting single File
Deleting Multiple Files