Implement drag and drop for maintable #3765

Siedlerchr · 2018-02-22T21:53:36Z

Drag And Drop Functionality

Fixes #4086
Fixes #3923
Fixes #3258
Fixes #4087

Current behaviour Windows: Shift or no modifer on maintable (move)

Create new entry from XMP content (if pdf)
- XMP content found: Show dialog with content and option to import entry or to create file
  ALT on maintable: move file to file dir and link (only windows)
  CTRL on maintable: copy file to file dir (if not exists) and link
  Bib-File dragged to maintable: Import contents

Xubuntu: No modifier (Copy)

Create new entry from XMP content (if pdf)
- XMP content found: Show dialog with content and option to import entry or to create file
  Shift on maintable : move to file dir

Preview Panel works the same as above except it doesn't handle bib files import,

BibFile dragged below tab pane/table header -> Open bib libray

General Tab Files -> Link file

Change in CHANGELOG.md described
Tests created for changes
Manually tested changed features in running JabRef
Screenshots added in PR description (for bigger UI changes)
Ensured that the git commit message is a good one
Check documentation status (Issue created for outdated help page at help.jabref.org?)

TODO: DataFormat - Whole entries?

tobiasdiez · 2018-02-22T23:22:19Z

Cool that you have a look at the drag'n'drop functionality. It's indeed one of the major things missing in the new maintable.
I think, there are generally three different scenarios:

Source outside of JabRef, Target maintable: This may be a pdf file (-> import as new entry), some string (-> try to parse/import) or a bib file (-> import and if target is not the maintable but the tab pane -> open as new library).
Target outside of JabRef, Source maintable: For this a string representation is needed (since no external application can handle BibEntries) but I'm not sure what we should serialize. The bibtex code seems like the logical choice but also the preview format (e.g. drag'n'drop to email program) or \cite{key} (target latex editor) makes sense. Maybe add a user preference?
Target and source are JabRef nodes: Then it is in general desirable to not serialize the entry but pass it by reference. For example, drag an entry from the maintable to the group pane; one would need the original entry to modify the groups field. Not sure if we need drag'n'drop from maintable to maintable (what's the use for this?). For the implementation of these local operations by reference, I would use the following code as a starting point: https://community.oracle.com/message/10909582#10909582 (slightly modified to use Optionals instead of nulls).

Siedlerchr · 2018-02-23T08:09:04Z

I implemented drag and drop a while ago for the Linked Files editor, so I know now to add serialization. Drag and drop from and to maintable could be used for reordering entries. But not sure if that makes sense. I just implemented it to test if it works in general

lenhard · 2018-02-23T09:04:59Z

I agree with everything that @tobiasdiez writes. But just for the record: Turning BibEntry serializable is trivial. It's just a marker interface and there is no functionality to add. Just a few attributes should be marked as transient and that's it. Nevertheless, I think it's better to serialize the BibTeX String representation.

Siedlerchr · 2018-02-23T11:35:29Z

@lenhard In theory es, but in practice I now stumbled across the problem (again) that those javafx ObservableMaps/List/Sets etc can't be serialized. So I tried adding custom readObject/writeObject methods, but still can't serialize it, so I would prefer sticking with the parser/serialization.

Ideally we would have a method which is the exact opposite of the serializeAll() - e.g. deserialzeAll() without having to use the parser overhead. Or maybe a simple toString() - reverse

And I come to the conclusion that this is not the best idea and we should use some trans

tobiasdiez · 2018-02-23T13:32:34Z

Yes, the javafx properties are never serializable because you cannot serialize listeners (they same applies to the event bus). But as I argued above, the best is to never serialize any Java object if drag'n'drop happens between nodes in JabRef itself. Just store a reference to the objects that are dragged.

Siedlerchr · 2018-02-23T14:17:20Z

Okay, I will try with that LocalClipboard/DragBoard Implementation
Refs #121

Add LocalDragboard

Siedlerchr · 2018-02-23T21:05:51Z

src/main/java/org/jabref/model/database/BibDatabase.java

@@ -116,7 +116,7 @@ public boolean containsEntryWithId(String id) {
    }

    public ObservableList<BibEntry> getEntries() {
-        return FXCollections.unmodifiableObservableList(entries);
+        return entries;


@tobiasdiez Is there a reason why the entries are all wrapped in an Unmodiefable collection?

Yes, so that nobody can add entries to the database without calling insertEntry which also does some bookkeeping (id list, tex free version, ...). I think the general principle is that objects should never expose their inner data.

Siedlerchr · 2018-02-23T21:06:53Z

src/main/java/org/jabref/gui/maintable/MainTable.java


-        ObservableList<BibEntryTableViewModel> items = this.itemsProperty().get();
+            BibEntry entry = this.getItems().get(draggedIndex).getEntry();


I followed the guide in https://stackoverflow.com/a/28606524/3450689
@tobiasdiez How do I implement the insert/delete at best? The problem is that all those ViewModel stuff is wrapped in many Unmodiefiable/ReadOnly Collections which don't support remove/insert

And calling database.removeEntry/insertEntry everytime I just do drag and drop on the maintable seems to be a bit overhead (as all kind of events are triggering)
Or do we just don't allow reordering the MainTable?

I think reordering of entries is a delicate business: what does it mean to reorder entries if I sort the table by author? The order of the entries in the bib file is determined independently anyway. For these reasons, I would not try to implement reordering of entries.

Rename getEntriesFiltered to better distinguish it

…drop * upstream/maintable-beta: Save order of columns across sessions (#3783) Allow side pane to be completely hidden (#3784) Show empty group pane if no database is open (#3785) Reenable drag'n'drop support for tabs / libraries (#3688)

…drop * upstream/maintable-beta: (24 commits) Fixes #3796: Pretend that we have every translation for every key (#3820) adjust line wrapping on column replace open office install selection dialog with choice dialog Wrap error messages in fx executor (because abstract worker is swing thread) Convert swing error dialog in save db action to custom fx dialog Replace swing export dialog in ExportToClipboardAction with fx choices dlg run find fulltext dialog in fx thread fix cut, copy & paste action from toolbar Set resizable for custom dialog with dialog pane convert merge entries dlg to javafx and make action working again checkstyle argh! Update test order -> Execute checkstyle first (#3807) embed cleanup dialog in javafx Fix some non FX Thread issues Main drawback: Cleanup panel is in backgroung due to swing thread) Allow search field to be smaller Fix Codacy Unused Params, Fields (#3753) Dialogstojavafx (#3801) Remove unused import Add group coloring in maintable as replacement for marked entries (#3769) Enable travis build for maintable-beta (#3804) Remove setting of tooltip manually Change open last edited dialgo to javafx (#3800) ... # Conflicts: # src/main/java/org/jabref/gui/maintable/MainTable.java

Add files and move them to file directory via cleanup operation Remove unsused parameter LayoutFormatterPreferences from Cleanup

…drop * upstream/maintable-beta: (97 commits) Update Eclipse style to intellij style (#3827) fix some not on fx thread dialogs in preferences fix inital save error Convert last dialog to javafx refactor exception Merge changes for renamed actions New Crowdin translations (#3835) Partial revert of #3715 since double click on group should expand/collapse it (#3834) Add Spanish translations (#3833) update applicationsinsights update javafxsvg to 1.3.0 Disable FTP Download on CI Apply copy linked files dialog fix from master Show dialog when copy files did not found file (#3826) Remove customjfx (#3779) Disable FTP Download on CI Remove color customization for maintable from preferences (#3808) Remove erroneous escape character (#3831) New translations JabRef_en.properties (French) New translations JabRef_en.properties (Tagalog) New translations JabRef_en.properties (Italian) New translations JabRef_en.properties (Indonesian) ... # Conflicts: # src/main/java/org/jabref/gui/maintable/MainTable.java

…drop * upstream/maintable-beta: Update java-string-similarity from 1.0.1 -> 1.1.0 Update log4j from 2.10.0 -> 2.11.0 Update guava from 24.0 -> 24.1 New Crowdin translations (#3853) Fetcher refactorings Fix ScienceDirect fetcher #3854 Fix Google Scholar fetcher test Fix DBLP fetcher tests Move migration to top level package New Crowdin translations (#3851) Update mysql-connector from 5.1.45 -> 5.1.46 Update mockito-core from 2.15.0 -> 2.16.0 New translations JabRef_en.properties (Spanish) New translations Menu_en.properties (Spanish) First available entry selected after pressing ESC as well Pressing ESC now clears search field

Add missing keybinding

…drop * upstream/maintable-beta: (48 commits) Clean unused imports Fix missing icons and wrong package for custom icons Consistent FX color scheme for JabRef (#3839) javafx replacement for file dialog (#3005) Reenable closing of entry preview by pressing Esc (#3883) Load all field editors using ViewLoader Use JabRef icons in FXML Move icon stuff to new package gui.icon Load EntryEditor using new ViewLoader Improve tooltip tests Fix import thread problem Don't use null as parameter in DialogService Make it easier to create FXML dialogs (#3880) update slf4j from 1.8.0-beta1 -> 1.8.0-beta2 New translations JabRef_en.properties (Tagalog) New translations JabRef_en.properties (Italian) New translations Menu_en.properties (Italian) New translations JabRef_en.properties (Indonesian) New translations JabRef_en.properties (Greek) New translations Menu_en.properties (Greek) ... # Conflicts: # src/main/java/org/jabref/gui/actions/CleanupAction.java # src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java # src/main/java/org/jabref/gui/groups/GroupTreeController.java # src/main/java/org/jabref/gui/maintable/MainTable.java # src/main/java/org/jabref/logic/cleanup/MoveFilesCleanup.java # src/main/java/org/jabref/logic/cleanup/RenamePdfCleanup.java # src/test/java/org/jabref/logic/cleanup/MoveFilesCleanupTest.java # src/test/java/org/jabref/logic/cleanup/RenamePdfCleanupTest.java

fix moving if not pdf

Siedlerchr · 2018-07-30T09:51:04Z

I tested and updated the results with my observance under Xubuntu. No modifier under Ubuntu equls TransferMode.Copy
Shift on Xubuntu: Transfermode.Move

Windows: No modifier and or shift: Transfermode.Move
CTRL Modifier : Transfermode Copy

* upstream/master: (33 commits) Fix display of checkboxes Fix Remove Spin comments Fix#3861_build_addneeded/deleteobsoleteproperties2 Fix#3861_build_addneeded/deleteobsoleteproperties Fix#3861_build Fix attach file does not relativize file path (#4252) Fix for issue koppor 254 (Auto cleanup url) (#4255) Fix#ReplaceString_ChangeConflict Fix#3861ReplaceString_solveConflict Fix#3861RS_conflicttest Fix#3861RS_conflicttest Remove changelog entries Fix#3861ReplaceString_checkStyle Fix#3861ReplaceString_Improvements Fix#3861ReplaceString_ContributeLog fix log4j slf4j dependency Update dependencies (#4251) Fix author list parser (#4169) (#4248) ReplaceStringMVVMPattern ... # Conflicts: # src/main/java/org/jabref/gui/BasePanel.java

remove some println

Siedlerchr · 2018-08-13T10:41:14Z

@tobiasdiez I still don't really get how I the NewDroppedFileHandler could be changed, because they all act on dropped files. Either they import the entries or if no entries are found the file is added. So I need both methods, or if I extract the latter ones to a new class, I still would need to call the methods for moving files. Maybe you have a better idea

tobiasdiez · 2018-08-13T12:30:34Z

My proposal was to split it as follows:

EntryFileLinker: provides a bunch of methods that allow to move/copy/link files to a given BibEntry (which was passed as a constructor argument)
DatabaseFileLinker: allows to add files to a given database (passed as constructor argument) and generate BibEntries out of the content. You are of course allowed to reuse EntryFileLinker if this is convenient.
GUI stuff for the dialogs

(the names for the classes are not very good). The first two classes should reside in logic. But maybe I miss something why such a splitting is not possible.

* upstream/master: Fix for issue 3959: migrate all tests to JUnit 5 (#4260) Fix "Manage journal abbreviations" dialog (#4263) # Conflicts: # src/test/java/org/jabref/logic/cleanup/MoveFilesCleanupTest.java # src/test/java/org/jabref/logic/cleanup/RenamePdfCleanupTest.java

tobiasdiez

I know it is not yet marked as ready for review but I nonetheless went through the code since this is an important feature that we should get in as soon as possible. In general the code looks good and my remarks are mostly suggestions for small code cleanups.

tobiasdiez · 2018-08-17T10:27:23Z

src/main/java/org/jabref/gui/BasePanel.java

@@ -1312,11 +1312,13 @@ private void saveDividerLocation(Number position) {
        }
    }

+    /*


tobiasdiez · 2018-08-17T10:30:44Z

src/main/java/org/jabref/gui/JabRefFrame.java

+        });
+
+        tabbedPane.setOnDragDropped(event -> {
+            System.out.println("drag drop in tabbed pane" + event.getGestureTarget());


remove debug statement

tobiasdiez · 2018-08-17T10:31:31Z

src/main/java/org/jabref/gui/JabRefFrame.java

@@ -215,6 +219,30 @@ private void init() {

        initKeyBindings();

+        tabbedPane.setOnDragOver(event -> {
+            if (event.getDragboard().hasFiles() && (event.getSource() instanceof DndTabPane)) {


What's the purpose of the event.getSource() instanceof DndTabPane check?

No longer needed.

tobiasdiez · 2018-08-17T10:32:38Z

src/main/java/org/jabref/gui/JabRefFrame.java

-                );
+                                  factory.createIconButton(StandardActions.FORK_ME, new OpenBrowserAction("https://github.com/JabRef/jabref")),
+                                  factory.createIconButton(StandardActions.OPEN_FACEBOOK, new OpenBrowserAction("https://www.facebook.com/JabRef/")),
+                                  factory.createIconButton(StandardActions.OPEN_TWITTER, new OpenBrowserAction("https://twitter.com/jabref_org")));


Please leave the ); on the new line since this grouping makes reading the code easier (applies also to the changes below)

tobiasdiez · 2018-08-17T10:35:16Z

src/main/java/org/jabref/gui/PreviewPanel.java

+            boolean success = false;
+            if (event.getDragboard().hasContent(DataFormat.FILES)) {
+                List<Path> files = event.getDragboard().getFiles().stream().map(File::toPath).collect(Collectors.toList());
+                System.out.println(files);


Remove log statements here and everywhere

tobiasdiez · 2018-08-17T10:52:43Z

src/main/java/org/jabref/gui/maintable/MainTable.java

+
+                    System.out.println("Mode MOVE"); //shift on win or no modifier
+
+                    fileHandler.addNewEntryFromXMPorPDFContent(entry, files);


Again, I would find it logical here to move and link the file.

tobiasdiez · 2018-08-17T10:56:13Z

src/main/java/org/jabref/gui/util/CustomLocalDragboard.java

+ */
+public class CustomLocalDragboard {
+
+    private final Map<Class<?>, Object> contents = new HashMap<>();


We know what types we want to put in our dragboard and these are only a hand-full (like entries and groups). Thus, I would prefer a strongly-typed version of this class exposing methods like put/getEntries and put/getGroups.

With this version we have a complete generic type safe local dragboard. Implementation for groups and bib entries would be the essentially the same code so I decided to use a generic type safe approach.In addition provides possibilities for all other kind of types

There speaks nothing against leaving the generic Map here, but personally I find putValue(DragAndDropDataFormats.BIBENTRY_LIST_CLASS, entries) harder to read than putEntries(entries). Thus, my suggestion was to add overloads for the common use cases (similar to how the javafx dragboard has get/has/putFiles methods).

tobiasdiez · 2018-08-17T10:59:24Z

src/main/java/org/jabref/logic/externalfiles/ExternalFilesEntryLinker.java

+    public void addFilesToEntry(BibEntry entry, List<Path> files) {
+
+        for (Path file : files) {
+            FileUtil.getFileExtension(file).ifPresent(ext -> {


Even if we cannot get the file extension, we should still add the file (but with empty type).

tobiasdiez · 2018-08-17T11:00:33Z

src/main/java/org/jabref/model/entry/BibEntry.java

@@ -57,14 +57,15 @@
     * Cache that stores latex free versions of fields.
     */
    private final Map<String, String> latexFreeFields = new ConcurrentHashMap<>();
-    private final EventBus eventBus = new EventBus();
+    private final transient EventBus eventBus = new EventBus();


no longer needed right?

tobiasdiez · 2018-08-17T11:01:12Z

src/main/java/org/jabref/model/entry/SharedBibEntryData.java

 /**
 *  Stores all informations needed to manage entries on a shared (SQL) database.
 */
-public class SharedBibEntryData {
+public class SharedBibEntryData implements Serializable {


̀Serializable should also no longer be needed.

reworked debug logging pass customlocaldragboard as parameter to groups

move from logic to gui because of the externalFileType stuff add property to speedup rendering pdf content move and rename

clear clipboard on put fix drag and drop to groups by adding clipboard stuff

tobiasdiez · 2018-08-19T20:33:35Z

Thanks for the quick follow-up. Merging, since I'm somewhat trigger happy right now ;-)

* upstream/master: (129 commits) Fix Export to clipbaord action (#4286) Add minimal height for entry editor (#4283) Fix exceptions when trying to change settings (#4284) Added a comment for the purpose of the class (#4282) Fix NPE when importing from CLI (#4281) Fix display of key patterns convert bibtexkeypatterndlg to javafx as well (#4277) Style preference sections using css Fix a few style issues Changed the behavior of the field formatting dialog (#4275) Fix checkbox display Remove small font size Style side pane in preferences Wrap preference tabs in scrollpane Fix NPE with bibdatabasecontext in preview style dialog update xmlunit and log4j jul and checkstyle Update mysql driver to 8.0.12 Rework preference dialog main class Move preferences stuff to gui.preferences Implement drag and drop for maintable (#3765) ... # Conflicts: # src/main/java/org/jabref/gui/shared/ConnectToSharedDatabaseDialog.java

Siedlerchr added 2 commits February 22, 2018 22:53

Implement drag and drop for maintable

c29344b

TODO: DataFormat - Whole entries?

Use BibtexWriter to serialize all entries and parser to deserialize

80a3a63

try further around with serializing and deserializing

8f17758

koppor mentioned this pull request Feb 23, 2018

Interface freezes when dragging and dropping entries to groups after adding entries #3304

Closed

Siedlerchr added 3 commits February 23, 2018 17:59

Add new Dataformat for List<BibEntry>

bb14f03

Add LocalDragboard

Add DragAndDropDataFormat

d02f180

drag and drop to groups works now

cc2c1d2

tobiasdiez mentioned this pull request Feb 23, 2018

[WIP] Main table JavaFX migration #3621

Merged

35 tasks

Prepare File Dropping

9eee5ae

Siedlerchr commented Feb 23, 2018

View reviewed changes

No reordering of items in the maintable via drag and drop

9989f9e

Rename getEntriesFiltered to better distinguish it

Siedlerchr force-pushed the fixdragandrop branch from 924e06f to 9989f9e Compare February 23, 2018 21:29

add basic support for adding droppped file to entry

b25151e

Siedlerchr added 9 commits February 28, 2018 17:32

add check for transfer mode move and link

d8ee0e8

Add link to file handler

adf4cba

Add files and move them to file directory via cleanup operation Remove unsused parameter LayoutFormatterPreferences from Cleanup

add dialog for importing entries from xmp

0ebdcfb

prepare pdf content importer code

d4a5af4

Add missing keybinding

Siedlerchr added 3 commits July 28, 2018 20:33

fix checkstyle

177b7ad

fix l10n

37e354c

fix copying on linux

9f804d8

fix moving if not pdf

Siedlerchr added 2 commits August 13, 2018 11:54

remove old drag and drop popup classes

c10d838

remove some println

fix import order

58e5bcc

Siedlerchr added 3 commits August 16, 2018 11:43

extract part to logic

57aa79a

extract files adding and content importer to own class

f1f1d91

tobiasdiez requested changes Aug 17, 2018

View reviewed changes

Siedlerchr and others added 8 commits August 17, 2018 17:44

some cleanups

8fcaf66

reworked debug logging pass customlocaldragboard as parameter to groups

Add fileUtil method for checking if it's a bib file

ab1b90a

move from logic to gui because of the externalFileType stuff add property to speedup rendering pdf content move and rename

fix l10n

adf1095

cleanup

e2c6c0e

add methods for putting and getting bib entries of localdragboard

dfdc6dc

clear clipboard on put fix drag and drop to groups by adding clipboard stuff

add javadoc

259d5fd

Merge branch 'master' into fixdragandrop

f2de954

Fix build

44b7524

tobiasdiez merged commit 1f14795 into master Aug 19, 2018

tobiasdiez deleted the fixdragandrop branch August 19, 2018 20:33

Siedlerchr mentioned this pull request Aug 20, 2018

Fix NPE with bibdatabasecontext in preview style dialog #4278

Merged

6 tasks

Siedlerchr mentioned this pull request Aug 29, 2018

Dropping a PDF on an entry should be consistent #121

Closed

Siedlerchr mentioned this pull request Jun 3, 2019

where to drop PDF to import as new entry? #5017

Closed

Siedlerchr mentioned this pull request Nov 1, 2019

Make entry editor DND behave as specified in settings #5554

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement drag and drop for maintable #3765

Implement drag and drop for maintable #3765

Siedlerchr commented Feb 22, 2018 •

edited

Loading

tobiasdiez commented Feb 22, 2018 •

edited

Loading

Siedlerchr commented Feb 23, 2018

lenhard commented Feb 23, 2018

Siedlerchr commented Feb 23, 2018

tobiasdiez commented Feb 23, 2018

Siedlerchr commented Feb 23, 2018 •

edited

Loading

Siedlerchr Feb 23, 2018 •

edited

Loading

tobiasdiez Feb 23, 2018

Siedlerchr Feb 23, 2018

tobiasdiez Feb 23, 2018

Siedlerchr commented Jul 30, 2018

Siedlerchr commented Aug 13, 2018

tobiasdiez commented Aug 13, 2018

tobiasdiez left a comment

tobiasdiez Aug 17, 2018

tobiasdiez Aug 17, 2018

tobiasdiez Aug 17, 2018

Siedlerchr Aug 17, 2018

tobiasdiez Aug 17, 2018

tobiasdiez Aug 17, 2018

tobiasdiez Aug 17, 2018

tobiasdiez Aug 17, 2018

Siedlerchr Aug 17, 2018

tobiasdiez Aug 18, 2018

tobiasdiez Aug 17, 2018

tobiasdiez Aug 17, 2018

tobiasdiez Aug 17, 2018

tobiasdiez commented Aug 19, 2018


		ObservableList<BibEntryTableViewModel> items = this.itemsProperty().get();
		BibEntry entry = this.getItems().get(draggedIndex).getEntry();

@@ @@ -1312,11 +1312,13 @@ private void saveDividerLocation(Number position) { @@
                       }
                   }
+                  /*


		System.out.println("Mode MOVE"); //shift on win or no modifier

		fileHandler.addNewEntryFromXMPorPDFContent(entry, files);

Implement drag and drop for maintable #3765

Implement drag and drop for maintable #3765

Conversation

Siedlerchr commented Feb 22, 2018 • edited Loading

tobiasdiez commented Feb 22, 2018 • edited Loading

Siedlerchr commented Feb 23, 2018

lenhard commented Feb 23, 2018

Siedlerchr commented Feb 23, 2018

tobiasdiez commented Feb 23, 2018

Siedlerchr commented Feb 23, 2018 • edited Loading

Siedlerchr Feb 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Siedlerchr commented Jul 30, 2018

Siedlerchr commented Aug 13, 2018

tobiasdiez commented Aug 13, 2018

tobiasdiez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiasdiez commented Aug 19, 2018

Siedlerchr commented Feb 22, 2018 •

edited

Loading

tobiasdiez commented Feb 22, 2018 •

edited

Loading

Siedlerchr commented Feb 23, 2018 •

edited

Loading

Siedlerchr Feb 23, 2018 •

edited

Loading