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 null checks in GroupChangeViewModel #3529

Merged
merged 2 commits into from
Dec 16, 2017
Merged

Add null checks in GroupChangeViewModel #3529

merged 2 commits into from
Dec 16, 2017

Conversation

lenhard
Copy link
Member

@lenhard lenhard commented Dec 14, 2017

This avoids the NPEs in #3526. However, I can't get the groups side pane to repaint and depict the modified groups tree. I've tried different ways via calling the GroupsSidePane or setting selected groups in the StateManager, but no success.

@tobiasdiez How do I force the GroupSidePane to display a changed group tree?


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

@tobiasdiez
Copy link
Member

Sadly updates to the group tree in general are ignored, see e.g. #3189. Normally, one would like that all changes are automatically propagated using the event bus / javafx binding, but for some reason this does not work (sorry, I forgot the details).

@lenhard
Copy link
Member Author

lenhard commented Dec 14, 2017

Ok, thanks for clarifying. I am not sure what to do with this PR then. Under the hood it works (avoids the exceptions), but all of this is not visible to the user. Right now it requires a restart of the application.

@tobiasdiez
Copy link
Member

Is the view updated when you switch between databases? This always worked for me so far. I would say merge this PR here (since it fixes the NPE) and comment in the above issue that there are more update problems. I'll have a look at this problem, but not right now as this week is kind of a hell for me.

@lenhard
Copy link
Member Author

lenhard commented Dec 14, 2017

Yes, it is updated when switching between databases, e.g. all groups are removed and the all entries group is displayed when I remove the group statement. A rename of the group in the file doesn't carry over, so there are probably deeper problems.

I still have the problem that it tends to display the groups tree of the wrong database, but at least the change is reflected somehow.

@Siedlerchr
Copy link
Member

Can this be merge now?

@lenhard
Copy link
Member Author

lenhard commented Dec 16, 2017

Sure, go ahead.

@tobiasdiez tobiasdiez merged commit 1af727b into master Dec 16, 2017
@tobiasdiez tobiasdiez deleted the fix3526 branch December 16, 2017 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants