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.

Sign up for GitHub

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

Jump to bottom

Add drag and drop for groups -> groups #2632

Merged
merged 7 commits into from
Mar 16, 2017
Merged

Add drag and drop for groups -> groups #2632

merged 7 commits into from
Mar 16, 2017

Conversation

tobiasdiez
Copy link
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
Copy link
Member

Siedlerchr commented Mar 14, 2017 •
edited
Loading

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

@matthiasgeiger
Copy link
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
Copy link
Member

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

@matthiasgeiger
Copy link
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
Copy link
Member

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

Siedlerchr
Siedlerchr reviewed Mar 14, 2017
View reviewed changes
src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java Outdated
@@ -67,7 +67,8 @@ public StringProperty filterTextProperty() {
* We need to notify the {@link StateManager} about this change so that the main table gets updated.
*/
private void onSelectedGroupChanged(GroupNodeViewModel newValue) {
if (currentDatabase != stateManager.activeDatabaseProperty().get()) {
//TODO: Can one of this be empty?
if (currentDatabase.get() != stateManager.activeDatabaseProperty().get().get()) {
Copy link
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

Copy link
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
Copy link
Member Author

Thanks @Siedlerchr for making the groups great again!

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

row.getItem().addEntriesToGroup(entrySelection.getSelection());
Copy link
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

Copy link
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
Copy link
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
Copy link
Member Author

tobiasdiez commented Apr 4, 2017 •
edited
Loading

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