Skip to content

Issue 9646: Right-click context menu "Attach file from URL"#9648

Merged
Siedlerchr merged 8 commits into
JabRef:mainfrom
bf:issue-9646
Mar 6, 2023
Merged

Issue 9646: Right-click context menu "Attach file from URL"#9648
Siedlerchr merged 8 commits into
JabRef:mainfrom
bf:issue-9646

Conversation

@bf

@bf bf commented Mar 4, 2023 •
edited by Siedlerchr
Loading

Copy link
Copy Markdown
Contributor

Fixes #9646

Add "Attach file from URL" to right-click context menu. Same functionality as the "download file" from the detail dialog.

right-click context menu:
image

url dialog:
image

file added:
image

  • 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 developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

@bf bf mentioned this pull request Mar 4, 2023
Closed
@bf

bf commented Mar 4, 2023

Copy link
Copy Markdown
Contributor Author

I had to duplicate some code from LinkedFilesEditorViewModel because of the way where taskexecutor and preferences come from. I now forward taskexecutor from RightClickMenu like other actions in that file are doing.

The one thing that AttachFileAction has and AttachFileFromURLAction doesn't is the undo() functionality. I was unsure how to implement it as in LinkedFilesEditorViewModel I couldn't find any change events that the undo system seems to need.

@bf bf changed the title Issue 9646 Issue 9646: Right-click context menu "Attach file from URL" Mar 4, 2023
@bf

bf commented Mar 4, 2023

Copy link
Copy Markdown
Contributor Author

god eclipse is so bad

@bf

bf commented Mar 4, 2023

Copy link
Copy Markdown
Contributor Author

Can you please fix the checkstyle for me. I can't seem to get it to work in Eclipse. Eclipse is so shitty it hurts me.

@Siedlerchr

Siedlerchr commented Mar 4, 2023

Copy link
Copy Markdown
Member

@bf Yes, Eclipse (although I use it myself) is not really compatible with the style we use in intellij (all others use intellij)

@bf

bf commented Mar 4, 2023

Copy link
Copy Markdown
Contributor Author

thank you so much!

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

Siedlerchr commented Mar 4, 2023

Copy link
Copy Markdown
Member

Regarding undo, you can ignore this for now. That's still an issue on our list to provide a better solution.
I'll try to take a closer look at the code tomorrow but so far it seems fine

Siedlerchr
Siedlerchr previously approved these changes Mar 6, 2023
View reviewed changes
@bf

bf commented Mar 6, 2023

Copy link
Copy Markdown
Contributor Author

Thanks for the kind refactor @Siedlerchr. I've been working with the most recent version since a few hours and it works well.

Only thing I noticed is that some downloads return a 403 (I think it was sciencedirect.com) because the website is protected by cloudflare. But the download is also failing via curl (chrome dev tools -> copy request as curl) due to lack of Cloudflare cookies. So I think this is an unrelated issue because it also happens with the existing "download from url" feature. Also it is out of scope for JabRef to circumvent CloudFlare protections.

Again, thanks for the kind collaborative environment on this project and your super-quick turnaround times. Really appreciate it.

@calixtus

calixtus commented Mar 6, 2023

Copy link
Copy Markdown
Member

Hi bf, I took the liberty to refactor your very neat PR just a little bit. I hope you don't mind. I just moved the method from the utility class as a static method to your new attach command.
Looks good to me!

@Siedlerchr Siedlerchr merged commit 51b36c1 into JabRef:main Mar 6, 2023
@Siedlerchr

Siedlerchr commented Mar 6, 2023

Copy link
Copy Markdown
Member

Many thanks again for your contribution! Looking forward for more ;)

Siedlerchr added a commit that referenced this pull request Mar 14, 2023
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into dependabot/gradle/o…
2cbbebb 
…rg.mariadb.jdbc-mariadb-java-client-3.1.0

* upstream/main: (357 commits)
  Fix syntax
  Add experimental Fetcher for Bibliotheksverbund Bayern with MarcXML parser (#9641)
  Update guidelines-for-setting-up-a-local-workspace.md
  Update guidelines-for-setting-up-a-local-workspace.md
  Bump org.tinylog:slf4j-tinylog from 2.6.0 to 2.6.1 (#9665)
  Bump apple-actions/import-codesign-certs from 1 to 2 (#9662)
  Bump com.puppycrawl.tools:checkstyle from 10.8.0 to 10.8.1 (#9661)
  Bump gittools/actions from 0.9.15 to 0.10.2 (#9663)
  Bump hmarr/auto-approve-action from 3.1.0 to 3.2.0 (#9664)
  Bump io.github.classgraph:classgraph from 4.8.156 to 4.8.157 (#9666)
  Bump org.tinylog:tinylog-api from 2.6.0 to 2.6.1 (#9667)
  Add option to open arks in the browser from an ark identifier (#9601)
  remove "jdk 19 does not work" (#9658)
  Fulltext fetcher for IACR eprints (#9651)
  Observable Preferences S (#9619)
  Issue 9646: Right-click context menu "Attach file from URL" (#9648)
  Improve the INSPIREFetcher in "Update with bibliographic information from the web" (#9645)
  Bump appleboy/ssh-action from 0.1.7 to 0.1.8 (#9653)
  Bump com.fasterxml.jackson.datatype:jackson-datatype-jsr310 (#9656)
  Bump com.puppycrawl.tools:checkstyle from 10.7.0 to 10.8.0 (#9655)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Siedlerchr Siedlerchr Siedlerchr approved these changes

@calixtus calixtus calixtus 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.

Context Menu: "Attach File from URL" / "Download File from URL"

3 participants

@bf @Siedlerchr @calixtus