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

Remove entries from prior groups if move operation is selected #3105

Merged
merged 3 commits into from
Aug 16, 2017

Conversation

lenhard
Copy link
Member

@lenhard lenhard commented Aug 15, 2017

Fixes #3101

There is one problem left to conclude this. It does the proper operation on the selected entries and updates the group tree. However, it does not update the coloring in the group tree. @tobiasdiez Could you give me a hint on how I can achieve this?

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

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 code looks good to me.

In order to update the status, you need to tell the group tree that the selected entries changed (not the selection changed but the properties of the entries).
The cleanest solution is to use an extractor here https://github.com/JabRef/jabref/blob/master/src/main/java/org/jabref/gui/StateManager.java#L37, see for example https://stackoverflow.com/questions/31687642/callback-and-extractors-for-javafx-observablelist.
However, an explicit StateManager.setSelectedEntries(StateManager.getSelectedEntries()) should also work.

@lenhard
Copy link
Member Author

lenhard commented Aug 15, 2017

Unfortunately, using the StateManager does not solve the coloring for me.

But to be honest, it seems rather broken anyway, irrespective of this PR. I have a database with 3 entries and 2 groups. The only way to get a coloring is to click an entry with a group in a newly started JabRef without opening the entry editor. That coloring never changes even if I click different entries. Does coloring actually work for you?

I think this PR is ready to go. Someone else should check the fix in a running JabRef. The coloring problem should be investigated in another issue/PR.

@lenhard lenhard changed the title [WIP] Remove entries from prior groups if move operation is selected Remove entries from prior groups if move operation is selected Aug 15, 2017
@lenhard lenhard added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Aug 15, 2017
@tobiasdiez
Copy link
Member

In general it works for me but I also noticed that sometimes the color is not updated. Probably related to #2857.

@LinusDietz LinusDietz merged commit 2ab908f into master Aug 16, 2017
@LinusDietz LinusDietz deleted the group-move branch August 16, 2017 09:30
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