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 group and subgroups" option #2587

Merged
merged 5 commits into from
Mar 3, 2017
Merged

Conversation

tobiasdiez
Copy link
Member

Issue #1407 is fixed in the process (long names are handled well by default in javafx, see screenshot below).
snipimage

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

Issue #1407 is fixed in the process
@tobiasdiez tobiasdiez added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Feb 24, 2017
@@ -407,13 +403,13 @@ private void showPopup(MouseEvent e) {
if (node.canBeEdited()) {
editGroupPopupAction.setEnabled(false);
addGroupPopupAction.setEnabled(false);
removeGroupAndSubgroupsPopupAction.setEnabled(false);
//removeGroupAndSubgroupsPopupAction.setEnabled(false);
Copy link
Member

Choose a reason for hiding this comment

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

Why only commented out?

Copy link
Member Author

Choose a reason for hiding this comment

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

The new button is always enabled which is not what you want. Thus this old code functions as a reminder for myself to change it.

removeGroupKeepSubgroupsPopupAction.setEnabled(false);
} else {
editGroupPopupAction.setEnabled(true);
addGroupPopupAction.setEnabled(true);
addGroupPopupAction.setNode(node);
removeGroupAndSubgroupsPopupAction.setEnabled(true);
//removeGroupAndSubgroupsPopupAction.setEnabled(true);
Copy link
Member

Choose a reason for hiding this comment

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

Why only commented out?

@lenhard
Copy link
Member

lenhard commented Feb 28, 2017

If I newly create groups in some bib file and delete them using the options, everything works.

However, I just tested this with our large group file and unfortunately it does not work. I right-click a group (for instance "Life" as the first group in the large bib file), select "Remove group and subgroups", click ok in the message box. Then, nothing happens and the groups stay as they are. There are no messages in the log.

Am I missing something? Can you maybe have a try with our huge bib file?

@lenhard
Copy link
Member

lenhard commented Mar 1, 2017

Looks like other people see a group-related performance degradation: #2561 (comment) So, this is most likely not specific to this PR.

@tobiasdiez
Copy link
Member Author

@lenhard I can't reproduce the problem with the huge bib file. Thus I will merge this PR for now and would ask you to open a new issue with more details of how to reproduce it.

@tobiasdiez tobiasdiez merged commit 79ff330 into master Mar 3, 2017
@tobiasdiez tobiasdiez deleted the groupRemoveAction branch March 3, 2017 20:49
Siedlerchr added a commit that referenced this pull request Mar 7, 2017
* upstream/master: (91 commits)
  fixed #2613 (#2623)
  Add MathSciNet as ID-based fetcher (#2621)
  Add icon + color and description to groups (#2612)
  Fixed wrong logger import (#2618)
  Cleanup window has a scrollbar now. (#2614)
  Added the locale to a newly created class
  Move ExportComparator and CustomExportList to the correct package (only used in preferences)
  Fixes displaying of Mr DLib recommendations (#2616)
  Fix title-related key patterns in BibtexKeyPatternUtil (#2610)
  Remove OrdinalsToSuperscriptFormatter from recommended biblatex save actions (#2601)
  Update pgjdbc to new major version
  Update Dependenices String Similary log4j wiremock mockito
  Fix exception when parsing groups which contain a top level group (#2611)
  Add "Remove group and subgroups" option (#2587)
  Fix #1104: group selection is preserved under tab change
  Fix several File Cleanup + Rename issues  (#2415)
  Fixed a small error in the code comments
  Implement #1904: filter groups (#2588)
  Braces checking followup (#2598)
  Improve braces checking (#2593)
  ...
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