Show Citation style also in entry preview in preferences#4121
Conversation
| @@ -126,8 +131,19 @@ private void setupLogic() { | |||
| DefaultTaskExecutor.runInJavaFXThread(() -> { | |||
|
|
|||
| PreviewPanel testPane = new PreviewPanel(null, null, Globals.getKeyPrefs(), Globals.prefs.getPreviewPreferences(), dialogService); | |||
There was a problem hiding this comment.
Here null is passed for BasePanel and bibdatabasecontext,
| PreviewPreferences p = Globals.prefs.getPreviewPreferences(); | ||
| p = new PreviewPreferences(p.getPreviewCycle(),indexStyle,p.getPreviewPanelDividerPosition(),p.isPreviewPanelEnabled(), p.getPreviewStyle(),p.getPreviewStyleDefault()); | ||
|
|
||
| testPane = new PreviewPanel(Globals.basePanel, new BibDatabaseContext(), Globals.getKeyPrefs(), p, dialogService); |
There was a problem hiding this comment.
Do you really need the base panel here? If you look at the line 133 where the PreviewPanel is initialized for the first time, there is null passed for the basePanel and the bibdatabasecontex. So I assume you don't need the Globals.basepanel hack here.
There was a problem hiding this comment.
Yes, I need the basepanel because in the PreviewPanel.java file checks if basePanel is present in order to get the citation otherwise it doesn't work.
At first I initialized it with null because if no citation style is selected generates a default citation. So that's why I did it that way, the other thing I could try is to write the same logic there without calling the function updateLayout in PreviewPanel but it'll end up with duplicated code. What do you think is the best approach?
There was a problem hiding this comment.
Ah yes, I now did take a closer look at the code here. Having a dependency on the BasePanel is not nice. Well, but looking at the code and trying out some things I see no other way as the CitationStyleCache is configured on the basePanel.
However, you can use the method here JabRefGUI.getMainFrame().getCurrentBasePanel() . so you don't need that dependency on base panel in globals.
There was a problem hiding this comment.
Thank you, I removed it from Globals and used getCurrentBasePanel instead and it worked.
|
Thank you very much for your contribution! 👍 If it works without the BasePanel hack, it looks good to me. Our internal standards require a second dev to review your code until it will be merged. |
Siedlerchr
left a comment
There was a problem hiding this comment.
See my comments about the BasePanel hack
tobiasdiez
left a comment
There was a problem hiding this comment.
A big thank you also from me. The code looks good, aside from a very minor remark. Please add a changelog entry and then we can merge!
| } | ||
| else { | ||
| int indexStyle = chosen.getSelectedIndex(); | ||
| PreviewPreferences p = Globals.prefs.getPreviewPreferences(); |
There was a problem hiding this comment.
Please don't use abbreviations as this makes the code harder to understand: p -> preferences.
There was a problem hiding this comment.
Sorry about that, I changed it to preferences, and added a changelog entry as well. Thank you.
* upstream/master: Show Citation style also in entry preview in preferences (#4121) # Conflicts: # src/main/java/org/jabref/gui/preftabs/PreviewPrefsTab.java
* upstream/master: Extract v4.x changelog (#4125) Saves and reloads window state on close and on open (#4124) Fix convert to bibtex moves contents of the file field (#4123) Opens the directory of the currently edited file when opening a new file (#4106) Show Citation style also in entry preview in preferences (#4121) Mac graphics (#4074) The textarea in the PersonsEditor is focused when the field is focused (#4112)
Fixes #3849
It works, now you can have a preview of a selected citation style. I am not sure if it is the correct way of programming it though. Let me now if I should do it in some other way or try a different approach.