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 more menu items to library context menu #6551

Merged
merged 8 commits into from
Jun 1, 2020

Conversation

Hollyqqqqq
Copy link
Contributor

@Hollyqqqqq Hollyqqqqq commented May 29, 2020

Fixes #6527

Before fixed:

image

After fixed:

1

The following are relative codes. I just add some menu items and some relative action classes to perform actions.
https://github.com/Hollyqqqqq/jabref/blob/cd2fc42237242773685a3b38c2021e07cdbade39/src/main/java/org/jabref/gui/JabRefFrame.java#L1085-L1099

  • Change in CHANGELOG.md described (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.

Copy link
Member

@calixtus calixtus left a comment

Choose a reason for hiding this comment

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

Looks very good so far, just one idea for an enhancement.

@@ -1316,6 +1323,31 @@ public void execute() {
}
}

private class CloseOthersDatabaseAction extends SimpleCommand {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe you can add a utility method in ActionHelper to check if there are multiple databases present? In that case the entry for this could be disabled if not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean that I only need to disable the menu items when there is only one databese opened but no need to hide these items?
Just like the following is OK?

image

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.

Nice!
Tests are nice, but for changes that only concern the UI they are not necessary.

One suggestion: Add also the "Open terminal here" (from View) and a new "Reveal in file explorer" option (that opens the file explorer with the library marked - we should already have the necessary code in place in the JabRefDesktop class if I'm not mistaken).

factory.createMenuItem(StandardActions.CLOSE_ALL_LIBRARIES, new CloseAllDatabaseAction()),
new SeparatorMenuItem(),
factory.createMenuItem(StandardActions.LIBRARY_PROPERTIES, new LibraryPropertiesAction(this, stateManager)),
factory.createMenuItem(StandardActions.MANAGE_CITE_KEY_PATTERNS, new BibtexKeyPatternAction(this, stateManager))
Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't add the "Manage cite key pattern". It's not that important and also not directly concerned with the library.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think what you said is very reasonable. I will try to do it.

@Siedlerchr Siedlerchr changed the title Fix for issue 6527 Add more menu items to library context menu May 30, 2020
@Hollyqqqqq
Copy link
Contributor Author

@tobiasdiez @calixtus I've changed context menu contents as you asked. Waiting for your reply^_^

@Hollyqqqqq Hollyqqqqq marked this pull request as ready for review May 31, 2020 08:22
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.

Looks good to me. Thanks!

@tobiasdiez tobiasdiez added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label May 31, 2020
@Siedlerchr
Copy link
Member

@Hollyqqqqq Please fix the checkstyle issues. You can impor the style for intellij and then hit a key to format the code
https://devdocs.jabref.org/getting-into-the-code/guidelines-for-setting-up-a-local-workspace#using-jabrefs-code-style

@Hollyqqqqq
Copy link
Contributor Author

Sorry for didn't check the checkstyle issues. I'm done now.

@tobiasdiez
Copy link
Member

I've quickly added the localization information that were still missing (see https://devdocs.jabref.org/getting-into-the-code/code-howtos#using-localization-correctly for the next time) and will merge now.

Thanks again for your contribution! Keep them coming 🏎️

@tobiasdiez tobiasdiez merged commit 41a3d08 into JabRef:master Jun 1, 2020
Siedlerchr added a commit that referenced this pull request Jun 6, 2020
* upstream/master: (110 commits)
  Fix DOI import in RIS Importer
  Rename bibtexkey (#6545)
  check if fields are empty
  fix comment checkstyle arght
  Fix XMP Exctrator not returning empty optional
  Fix Group hit counter calculation preferences (#6554)
  add changelog entry
  Make test concise
  Add test for getXmpMetadata with no description
  guidelines updated
  lint and hint for JEE
  Update eclipse setting
  Bump checkstyle from 8.32 to 8.33
  Bump classgraph from 4.8.78 to 4.8.80
  Add more menu items to library context menu (#6551)
  Squashed 'src/main/resources/csl-styles/' changes from 586e0b8..c5f14e2
  Correct return description in javadoc
  iterate to get all selected leaf nodes
  check if metadata has start and end tag
  Update RisImporterTest.java
  ...
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.

Add more menu items to tab context menu
4 participants