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

Fix drag and drop into empty library #7555

Merged
merged 4 commits into from
Mar 24, 2021
Merged

Fix drag and drop into empty library #7555

merged 4 commits into from
Mar 24, 2021

Conversation

tp-1000
Copy link
Contributor

@tp-1000 tp-1000 commented Mar 21, 2021

fixes #6851
Drag & Drop into empty library

Fixes issue preventing files from being "dragged & dropped" into an
empty library

Added a drag/drop handlers on the table-view to allow for files
to be drag & drop into an empty library
  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

tp-1000 added 2 commits March 21, 2021 00:01
@tp-1000
Fixes issue JabRef#6851
5ce7656 
	Fixes issue preventing files from being drag & dropped into an
	empty library

	Added a drag/drop handlers on the table-view to allow for files
	to be drag & drop into an empty library
@tp-1000
Update CHANGELOG.md
00fa5d7
tobiasdiez
tobiasdiez approved these changes Mar 21, 2021
View reviewed changes
Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

Code wise this looks good to me. Thanks! Have you tested that dropping on entries still works and takes precedence over the general table view drop handler (e.g. directly dropping on an entry adds a link to this entry)?

Also you have checkstyle errors, may I ask you to please fix them.

@tp-1000
Copy link
Contributor Author

tp-1000 commented Mar 21, 2021

The short answer, no I cannot drop a file on an entry and get it to add the file to that entry. But it never did. I checked all the the positions top+bottom and center, and only top and bottom add files but only as new entries. I was a little confused as to the intended behavior. I would be happy to investigate further. How is it intended to work?

tp-1000 added 2 commits March 21, 2021 11:53
@tp-1000
Fixes style issue in CHANGELOG.md
b1f38a9 
	Lined added to wrong section

	Line move to fix issue
@tp-1000
Another style fix
04becfa 
	extra space removed
@Siedlerchr
Copy link
Member

Dropping a file on an entry depends on the modifier key you pressed (alt, shift or ctrl).
There's a description in the help somehow at the linked files topic

@tp-1000
Copy link
Contributor Author

tp-1000 commented Mar 21, 2021

I'll check it out and give you an update

@Siedlerchr Siedlerchr changed the title Fix for issue 6851 Fix drag and drop into empty library Mar 21, 2021
@tp-1000
Copy link
Contributor Author

tp-1000 commented Mar 21, 2021

So I'm guessing my issue is a Mac OS transfer mode key thing or possibly a system security setting, but I'm unable to get "move" to trigger and "copy" does nothing. Only "link" adds a file to the entry (control + drag). To answer your original question the entries still take precedence, and work exactly the way they did without my fix.

With my fix:
Drag and drop on an entry does not create a new entry. It works exactly as it did before.

Let me know if you want me to investigate more, or require further changes.

tobiasdiez
tobiasdiez approved these changes Mar 21, 2021
View reviewed changes
Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

Ok, thanks for testing!

@Siedlerchr didn't we fix these drag & drop issues on Mac as well?

@Siedlerchr
Copy link
Member

@tp-1000 On mac you need to use the option key.
However, I can only get modus copy to work.
https://developer.apple.com/documentation/appkit/nsdragginginfo/1415966-draggingsourceoperationmask?language=objc

See the switch method in handleOnDragDropped

@Siedlerchr
Copy link
Member

Siedlerchr commented Mar 22, 2021 •
edited
Loading

I checked this with a minimal demo app and it seems like it's a bug again in javafx

@tp-1000
Copy link
Contributor Author

tp-1000 commented Mar 22, 2021 •
edited
Loading

That's no good. Well, thanks for follow up. Yeah, I saw how it was suppose to work, just wasn't working for me.

@calixtus
Copy link
Member

I checked this with a minimal demo app and it seems like it's a bug again in javafx

Is there a related open issue in the javafx bugtracker?

@Siedlerchr
Copy link
Member

I couldn't find one, so I submitted one yesterday evening. TL;DR: On Mac only Copy mode works

@Siedlerchr
Copy link
Member

As this issue on mac is not related to your changes, https://bugs.openjdk.java.net/browse/JDK-8227371
I am merging this

@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Mar 24, 2021
@Siedlerchr Siedlerchr merged commit 2ee9767 into JabRef:master Mar 24, 2021
Siedlerchr added a commit that referenced this pull request Mar 28, 2021
@Siedlerchr
Merge remote-tracking branch 'upstream/master' into writebackupToTemp
f99ba6f 
* upstream/master: (191 commits)
  Fix for issue 7416: font size of the preferences dialog does not update with the rest of the GUI. (#7509)
  Fix school/instituation is printed twice (#7574)
  Dsiable notarisation until we hae an account for JabRef e.V. (#7572)
  Fix citation keys unintentionally being overwritten on import (#7443)
  Fix AuthentificationPlugin not declared in mergedModule (#7570)
  Suggestions for changes in caching latex free authors (#7301)
  Add simple Unit Tests (#7542)
  Fix drag and drop into empty library (#7555)
  Bump richtextfx from 0.10.4 to 0.10.6 (#7563)
  Bump pdfbox from 2.0.22 to 2.0.23 (#7561)
  Bump org.eclipse.jgit (#7560)
  Bump fontbox from 2.0.22 to 2.0.23 (#7562)
  Bump guava from 30.1-jre to 30.1.1-jre (#7564)
  Bump xmpbox from 2.0.22 to 2.0.23 (#7565)
  Bump hmarr/auto-approve-action from v2.0.0 to v2.1.0 (#7566)
  Add gource (#7193)
  UI: Fix for group icon (#7552)
  Fix for issue 6487: Opening BibTex file (doubleclick) from Folder with spaces not working (#7551)
  add ability to insert arxivId (#7549)
  Fixed missing trigger for linked file operations (#7548)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tobiasdiez tobiasdiez tobiasdiez approved these changes

@Siedlerchr Siedlerchr Siedlerchr approved these changes

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.

Drag & drop of PDF: not on empty library
4 participants
@tp-1000 @Siedlerchr @calixtus @tobiasdiez