Skip to content

Add drag and drop for groups -> groups#2632

Merged
tobiasdiez merged 7 commits into
masterfrom
groupDragDrop
Mar 16, 2017
Merged

Add drag and drop for groups -> groups#2632
tobiasdiez merged 7 commits into
masterfrom
groupDragDrop

Conversation

@tobiasdiez

@tobiasdiez tobiasdiez commented Mar 11, 2017

Copy link
Copy Markdown
Member

Enable drag and drop in the group panel.
I also had to remove the "worker" aspect of the group worker, ie. now the entry list is updated immediately (this might lead to performance problems in bigger db's; but without this change JabRef would just hang after drop and down).

  • Change in CHANGELOG.md described
  • Tests created for changes
  • Screenshots added (for bigger UI changes)
  • Manually tested changed features in running JabRef
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)
  • If you changed the localization: Did you run gradle localizationUpdate?

@tobiasdiez tobiasdiez added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Mar 11, 2017
@Siedlerchr

Siedlerchr commented Mar 14, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

I will add my changes from the other PR to this one

@matthiasgeiger

matthiasgeiger commented Mar 14, 2017

Copy link
Copy Markdown
Member

@Siedlerchr Really? Isn't the scope of this PR different?
You are working on "assign entries to groups by drag-and-drop" whereas this PR deals with "restructure the groups tree with drag-and-drop".

However, it yours and @tobiasdiez's decision whether this makes sense from the technical aspect.

@Siedlerchr

Siedlerchr commented Mar 14, 2017

Copy link
Copy Markdown
Member

In fact it uses the same methods for listening/reaction of an DND Event. My changes would be simply an else case.

@matthiasgeiger

matthiasgeiger commented Mar 14, 2017

Copy link
Copy Markdown
Member

Allright, then forget my objections 😉

@Siedlerchr
Add support for drag and drop of entries to groups
ff63449
@Siedlerchr
Some formatting stuff
875678b
@Siedlerchr
Merge remote-tracking branch 'upstream/master' into groupDragDrop
6b56eff 
* upstream/master:
  Donate to JabRef!
  Issuestats.com is down
  Add link to logo creator
  Improve perfomance of opening a database that still used the old groups format (#2636)
  Set POSIX filepermissions to 644 (#2637)
  Fix #2629: Removing entry from group updates view properly
  Remove empty tooltip in group panel
  Refactored around the PDF Annotation Importer (#2624)
  Fix last import issue
  Fix imports and some reformatting
  Fix wrong import order
  Add missing svg icon
  RIP old icon
  Add new logo files (#2164)

# Conflicts:
#	src/main/java/org/jabref/gui/groups/GroupSelector.java
@Siedlerchr

Siedlerchr commented Mar 14, 2017

Copy link
Copy Markdown
Member

@tobiasdiez Drag and drop of entries works now as expected 🥇

Siedlerchr
Siedlerchr reviewed Mar 14, 2017
View reviewed changes
Comment thread src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java Outdated
private void onSelectedGroupChanged(GroupNodeViewModel newValue) {
if (currentDatabase != stateManager.activeDatabaseProperty().get()) {
//TODO: Can one of this be empty?
if (currentDatabase.get() != stateManager.activeDatabaseProperty().get().get()) {

@Siedlerchr Siedlerchr Mar 14, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tobiasdiez I am not sure if we should compare the values or if it is enough to check the object refs

@tobiasdiez tobiasdiez Mar 16, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think equals is the cleanest solution

@AEgit AEgit mentioned this pull request Mar 16, 2017
Closed
29 tasks
@tobiasdiez tobiasdiez mentioned this pull request Mar 16, 2017
Closed
@tobiasdiez tobiasdiez merged commit 589a30e into master Mar 16, 2017
@tobiasdiez tobiasdiez deleted the groupDragDrop branch March 16, 2017 16:47
@tobiasdiez

tobiasdiez commented Mar 16, 2017

Copy link
Copy Markdown
Member Author

Thanks @Siedlerchr for making the groups great again!

Siedlerchr
Siedlerchr reviewed Mar 16, 2017
View reviewed changes
Comment thread src/main/java/org/jabref/gui/groups/GroupTreeController.java
TransferableEntrySelection entrySelection = (TransferableEntrySelection) dragboard
.getContent(DragAndDropDataFormats.ENTRIES);

row.getItem().addEntriesToGroup(entrySelection.getSelection());

@Siedlerchr Siedlerchr Mar 16, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to set succes true in the entries else part

@Siedlerchr Siedlerchr Mar 16, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed to master

@matthiasgeiger

matthiasgeiger commented Apr 4, 2017

Copy link
Copy Markdown
Member

Short question here: Drag-and-drop generally works in current master - but the main table is not updated at once. I.e., after dropping an entry to the currently selected group it is still grayed out as an "non-member". After reselecting the group it is displayed correctly in the "floating" list of group members.

Is this a known problem? Or should I open an issue?

@tobiasdiez

tobiasdiez commented Apr 4, 2017 •
edited
Loading

Copy link
Copy Markdown
Member Author

Good catch! I added it to the list of open group-related problems: #2599

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Siedlerchr Siedlerchr Siedlerchr left review comments

Assignees

No one assigned

Labels

status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tobiasdiez @Siedlerchr @matthiasgeiger