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.

Sign up for GitHub

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

Show Citation style also in entry preview in preferences #4121

Merged
merged 4 commits into from
Jun 12, 2018
Merged

Show Citation style also in entry preview in preferences #4121

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9401f46
Fixes issue 3849
eso31 Jun 10, 2018
76ea46d
Fixes some checkstyle errors and changes a line to pass the tests
eso31 Jun 10, 2018
5dddcde
Removes BasePanel hack
eso31 Jun 11, 2018
dabbfcf
Adds changelog entry and changes a variable name
eso31 Jun 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/main/java/org/jabref/Globals.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.util.Optional;
import java.util.UUID;

import org.jabref.gui.BasePanel;
import org.jabref.gui.ClipBoardManager;
import org.jabref.gui.GlobalFocusListener;
import org.jabref.gui.StateManager;
Expand Down Expand Up @@ -38,6 +39,7 @@ public class Globals {
// In the main program, this field is initialized in JabRef.java
// Each test case initializes this field if required
public static JabRefPreferences prefs;
public static BasePanel basePanel;
/**
* This field is initialized upon startup.
* Only GUI code is allowed to access it, logic code should use dependency injection.
Expand Down
1 change: 1 addition & 0 deletions src/main/java/org/jabref/gui/BasePanel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ public BasePanel(JabRefFrame frame, BasePanelPreferences preferences, BibDatabas

this.preview = new PreviewPanel(this, getBibDatabaseContext(), preferences.getKeyBindings(), preferences.getPreviewPreferences(), dialogService);
frame().getGlobalSearchBar().getSearchQueryHighlightObservable().addSearchListener(preview);
Globals.basePanel = this;
}

public static void runWorker(AbstractWorker worker) throws Exception {
Expand Down
20 changes: 18 additions & 2 deletions src/main/java/org/jabref/gui/preftabs/PreviewPrefsTab.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.jabref.logic.citationstyle.CitationStyle;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.util.TestEntry;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.preferences.PreviewPreferences;

import com.google.common.primitives.Ints;
Expand Down Expand Up @@ -84,13 +85,15 @@ private void setupLogic() {
availableModel.removeElement(object);
chosenModel.addElement(object);
}
storeSettings();
});

btnLeft.addActionListener(event -> {
for (Object object : chosen.getSelectedValuesList()) {
availableModel.addElement(object);
chosenModel.removeElement(object);
}
storeSettings();
});

btnUp.addActionListener(event -> {
Expand All @@ -102,6 +105,7 @@ private void setupLogic() {
newSelectedIndices.add(newIndex);
}
chosen.setSelectedIndices(Ints.toArray(newSelectedIndices));
storeSettings();
});

btnDown.addActionListener(event -> {
Expand All @@ -115,6 +119,7 @@ private void setupLogic() {
newSelectedIndices.add(newIndex);
}
chosen.setSelectedIndices(Ints.toArray(newSelectedIndices));
storeSettings();
});

btnDefault.addActionListener(event -> layout.setText(Globals.prefs.getPreviewPreferences()
Expand All @@ -126,8 +131,19 @@ private void setupLogic() {
DefaultTaskExecutor.runInJavaFXThread(() -> {

PreviewPanel testPane = new PreviewPanel(null, null, Globals.getKeyPrefs(), Globals.prefs.getPreviewPreferences(), dialogService);
Copy link
Member

Choose a reason for hiding this comment

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

Here null is passed for BasePanel and bibdatabasecontext,

testPane.setFixedLayout(layout.getText());
testPane.setEntry(TestEntry.getTestEntry());
if (chosen.isSelectionEmpty()) {
testPane.setFixedLayout(layout.getText());
testPane.setEntry(TestEntry.getTestEntry());
}
else {
int indexStyle = chosen.getSelectedIndex();
PreviewPreferences p = Globals.prefs.getPreviewPreferences();
Copy link
Member

Choose a reason for hiding this comment

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

Please don't use abbreviations as this makes the code harder to understand: p -> preferences.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry about that, I changed it to preferences, and added a changelog entry as well. Thank you.

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);
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@eso31 eso31 Jun 11, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Thank you, I removed it from Globals and used getCurrentBasePanel instead and it worked.

testPane.setEntry(TestEntry.getTestEntry());
testPane.updateLayout(p);
}

DialogPane pane = new DialogPane();
pane.setContent(testPane);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/preferences/PreviewPreferences.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
public class PreviewPreferences {

private final List<String> previewCycle;
private final int previewCyclePosition;
private int previewCyclePosition;
private final Number previewPanelDividerPosition;
private final boolean previewPanelEnabled;
private final String previewStyle;
Expand Down