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.

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

Add Remove link context menu entry in file editor #2972

Merged
merged 1 commit into from
Jul 10, 2017
Merged

Conversation

Siedlerchr
Copy link
Member

Fix for #2968
image

  • 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?

@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jul 5, 2017
@Siedlerchr Siedlerchr requested a review from tobiasdiez July 5, 2017 14:16
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.

The changes in this PR look good to me, but please don't forget #2939 (comment) which in light of these changes is even more significant.

@@ -146,11 +146,15 @@ private ContextMenu createContextMenuForFile(LinkedFileViewModel linkedFile) {
deleteFile.setOnAction(event -> viewModel.deleteFile(linkedFile));
deleteFile.setDisable(linkedFile.getFile().isOnlineLink());

MenuItem deleteLink = new MenuItem(Localization.lang("Remove link"));
deleteLink.setOnAction(event -> viewModel.removeFileLink(linkedFile));
deleteLink.setDisable(linkedFile.getFile().isOnlineLink());
Copy link
Member

Choose a reason for hiding this comment

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

I think it also makes sense to enable it for online links.

Copy link
Member

@LinusDietz LinusDietz left a comment

Choose a reason for hiding this comment

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

LGTM

@Siedlerchr Siedlerchr merged commit d972f45 into master Jul 10, 2017
@Siedlerchr Siedlerchr deleted the removeFileLink branch July 10, 2017 13:38
Siedlerchr added a commit that referenced this pull request Jul 11, 2017
* upstream/master:
  Eclipse J
  Add switch indentation for Eclipse and add some new missing formatting options
  Check for different editions in the duplicate check (#2991)
  Add CheckStyle Check for Constants (final static) (#2992)
  Add Remove link context menu entry in file editor (#2972)
  Fix DiVA tests
  Remove <pre> tag from entries fetched using MathSciNet (#2990)
  Fix Brazilian Portugese language loading (#2981)
  Use sftp's symlink command to provide symlink to latest version
  Update gradle from 4.0 to 4.0.1
  Fix group storage (#2978)
  Fix keybindings in entry editor (#2971)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants