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

Fix for Issue #4437 - Some bugs in preference->Entry table columns #4546

Merged
merged 14 commits into from
Jan 12, 2019
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
- We fixed an issue where only one PDF file could be imported [#4422](https://github.com/JabRef/jabref/issues/4422)
- We fixed an issue where "Move to group" would always move the first entry in the library and not the selected [#4414](https://github.com/JabRef/jabref/issues/4414)
- We fixed an issue where an older dialog appears when downloading full texts from the quality menu. [#4489](https://github.com/JabRef/jabref/issues/4489)
- We fixed an issue where uncaught exceptions were logged but the user wasn't informed about their occurance. [#2288] (https://github.com/JabRef/jabref/issues/2288)
- We fixed an issue where the Field name is not editable [#4546](https://github.com/JabRef/jabref/issues/4546)
rachelwu21 marked this conversation as resolved.
Show resolved Hide resolved



Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,30 @@ public PersistenceVisualStateTable(final MainTable mainTable, JabRefPreferences
this.preferences = preferences;

mainTable.getColumns().addListener(this::onColumnsChanged);
mainTable.getColumns().forEach(col -> col.sortTypeProperty().addListener(obs -> updateColumnSortType(col.getText(), col.getSortType())));
mainTable.getColumns().forEach(col -> col.sortTypeProperty().addListener(obs ->
updateColumnSortType(col.getText(), col.getSortType())));
mainTable.getColumns().forEach(col -> col.widthProperty().addListener(obs -> onColumnWidthChange()));

}

private void onColumnWidthChange() {
Copy link
Member

Choose a reason for hiding this comment

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

This method appears to be identical to updateColumnPreferences below, or do I miss something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The onColumnWidthChange() was redundant and a brain fart on my part and I've removed it. Thanks for the head up. :)

List<String> columnNames = new ArrayList<>();
List<String> columnsWidths = new ArrayList<>();

for (TableColumn<BibEntryTableViewModel, ?> column : mainTable.getColumns()) {
if (column instanceof NormalTableColumn) {
NormalTableColumn normalColumn = (NormalTableColumn) column;

columnNames.add(normalColumn.getColumnName());
columnsWidths.add(String.valueOf(normalColumn.getWidth()));
}
}
if (columnNames.size() == columnsWidths.size()) {
preferences.putStringList(JabRefPreferences.COLUMN_NAMES, columnNames);
preferences.putStringList(JabRefPreferences.COLUMN_WIDTHS, columnsWidths);
}
}

private void onColumnsChanged(ListChangeListener.Change<? extends TableColumn<BibEntryTableViewModel, ?>> change) {
boolean changed = false;
while (change.next()) {
Expand Down Expand Up @@ -63,8 +83,9 @@ private void updateColumnPreferences() {
}
}

// Finally, we store the new preferences.
preferences.putStringList(JabRefPreferences.COLUMN_NAMES, columnNames);
preferences.putStringList(JabRefPreferences.COLUMN_WIDTHS, columnsWidths);
if (columnNames.size() == columnsWidths.size()) {
preferences.putStringList(JabRefPreferences.COLUMN_NAMES, columnNames);
preferences.putStringList(JabRefPreferences.COLUMN_WIDTHS, columnsWidths);
}
}
}
130 changes: 62 additions & 68 deletions src/main/java/org/jabref/gui/preferences/TableColumnsTab.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,8 @@
import org.jabref.model.entry.BibtexSingleField;
import org.jabref.preferences.JabRefPreferences;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

class TableColumnsTab extends Pane implements PrefsTab {

private static final Logger LOGGER = LoggerFactory.getLogger(TableColumnsTab.class);

private final JabRefPreferences prefs;
private boolean tableChanged;
private final TableView colSetup;
Expand All @@ -67,6 +62,10 @@ class TableColumnsTab extends Pane implements PrefsTab {
private final RadioButton preferUrl;
private final RadioButton preferDoi;
/*** begin: special fields ***/
private final Button addRow;
private final Button deleteRow;
private final Button down;
private final Button up;
private final CheckBox specialFieldsEnabled;
private final CheckBox rankingColumn;
private final CheckBox qualityColumn;
Expand All @@ -88,6 +87,7 @@ class TableColumnsTab extends Pane implements PrefsTab {
private final VBox listOfFileColumnsVBox;
private final ObservableList<TableRow> data;
private final GridPane builder = new GridPane();

/**
* Customization of external program paths.
*
Expand All @@ -96,81 +96,80 @@ class TableColumnsTab extends Pane implements PrefsTab {
public TableColumnsTab(JabRefPreferences prefs, JabRefFrame frame) {
this.prefs = prefs;
this.frame = frame;
this.data = FXCollections.observableArrayList(
new TableRow("entrytype",75),
new TableRow("author/editor",300),
new TableRow("title",470),
new TableRow("year",60),
new TableRow("journal",130),
new TableRow("bibtexkey",100));

/* Populate the data of Entry table columns */
List<String> prefColNames = this.prefs.getStringList(this.prefs.COLUMN_NAMES);
this.data = FXCollections.observableArrayList();
for (int i = 0; i < prefColNames.size(); i++) {
this.data.add(new TableRow(prefColNames.get(i)));
}

/* UI for Entry table columns */
colSetup = new TableView<>();
TableColumn<TableRow,String> field = new TableColumn<>(Localization.lang("Field name"));
TableColumn<TableRow,Double> column = new TableColumn<>(Localization.lang("Column width"));
colSetup.setEditable(true);
TableColumn<TableRow, String> field = new TableColumn<>(Localization.lang("Field name"));
field.setPrefWidth(400);
column.setPrefWidth(240);
field.setCellValueFactory(new PropertyValueFactory<>("name"));
field.setCellFactory(TextFieldTableCell.forTableColumn());
field.setEditable(true);
field.setOnEditCommit(
(TableColumn.CellEditEvent<TableRow, String> t) -> {
t.getTableView().getItems().get(
t.getTablePosition().getRow()).setName(t.getNewValue());
});
column.setCellValueFactory(new PropertyValueFactory<>("length"));
column.setOnEditCommit(
(TableColumn.CellEditEvent<TableRow, Double> t) -> {
t.getTableView().getItems().get(
t.getTablePosition().getRow()).setLength(t.getNewValue());
// Since data is an ObservableList, updating it updates the displayed field name.
this.data.set(t.getTablePosition().getRow(), new TableRow(t.getNewValue()));
// Update the User Preference of COLUMN_NAMES
List<String> tempColumnNames = this.prefs.getStringList(this.prefs.COLUMN_NAMES);
tempColumnNames.set(t.getTablePosition().getRow(), t.getNewValue());
this.prefs.putStringList(this.prefs.COLUMN_NAMES,tempColumnNames);
});

tableRows.clear();
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 understand the purpose of the tableRows field? It appears that it is always synced to the data collection anyway. If that's the case, could you please remove tableRows. Thanks.

Copy link
Contributor Author

@rachelwu21 rachelwu21 Jan 7, 2019

Choose a reason for hiding this comment

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

I took a look and it turned out it was redundant with data as you said. I've removed it.

tableRows.addAll(data);
colSetup.setItems(data);
colSetup.getColumns().addAll(field,column);
colSetup.getColumns().add(field);

final TextField addName = new TextField();
addName.setPromptText("name");
addName.setMaxWidth(field.getPrefWidth());
addName.setPrefHeight(30);
final TextField addLast = new TextField();
addLast.setMaxWidth(column.getPrefWidth());
addLast.setPromptText("width");
addLast.setPrefHeight(30);
BorderPane tabPanel = new BorderPane();
ScrollPane sp = new ScrollPane();
sp.setContent(colSetup);
tabPanel.setCenter(sp);

HBox toolBar = new HBox();
Button addRow = new Button("Add");
addRow = new Button("Add");
addRow.setPrefSize(80, 20);
addRow.setOnAction( e -> {
if (!addLast.getText().isEmpty()) {
TableRow tableRow = addLast.getText().matches("[1-9][0-9]") ? new TableRow(addName.getText(), Integer.valueOf(addLast.getText())) : new TableRow(addName.getText());
addName.clear();
addLast.clear();
data.add(tableRow);
addRow.setOnAction(e -> {
TableRow tableRow = new TableRow(addName.getText());
addName.clear();
data.add(tableRow);
tableRows.clear();
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be sufficient to add the TableRow only to the data. The TavleView should update automatically.

tableRows.addAll(data);
colSetup.setItems(data);
tableChanged = true;
colSetup.refresh();

});

deleteRow = new Button("Delete");
deleteRow.setPrefSize(80, 20);
deleteRow.setOnAction(e -> {
if (colSetup.getFocusModel() != null && colSetup.getFocusModel().getFocusedIndex() != -1) {
tableChanged = true;
int row = colSetup.getFocusModel().getFocusedIndex();
TableRow tableRow = data.get(row);
Copy link
Member

Choose a reason for hiding this comment

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

Here it should also be sufficient to just change data without any further updates to colSetup.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed it now.

data.remove(tableRow);
tableRows.clear();
tableRows.addAll(data);
colSetup.setItems(data);
tableChanged = true;
colSetup.refresh();
}
});

Button deleteRow = new Button("Delete");
deleteRow.setPrefSize(80, 20);
deleteRow.setOnAction(e -> {
if (colSetup.getFocusModel() != null && colSetup.getFocusModel().getFocusedIndex() != -1) {
tableChanged = true;
int row = colSetup.getFocusModel().getFocusedIndex();
TableRow tableRow = data.get(row);
data.remove(tableRow);
tableRows.clear();
tableRows.addAll(data);
colSetup.setItems(data);
colSetup.refresh();
}});
Button up = new Button("Up");
up = new Button("Up");
up.setPrefSize(80, 20);
up.setOnAction(e-> {
up.setOnAction(e -> {
if (colSetup.getFocusModel() != null) {
int row = colSetup.getFocusModel().getFocusedIndex();
if (row > data.size() || row == 0) {
Expand All @@ -188,9 +187,9 @@ public TableColumnsTab(JabRefPreferences prefs, JabRefFrame frame) {
return;
}
});
Button down = new Button("Down");
down = new Button("Down");
down.setPrefSize(80, 20);
down.setOnAction(e-> {
down.setOnAction(e -> {
if (colSetup.getFocusModel() != null) {
int row = colSetup.getFocusModel().getFocusedIndex();
if (row + 1 > data.size()) {
Expand All @@ -208,8 +207,9 @@ public TableColumnsTab(JabRefPreferences prefs, JabRefFrame frame) {
return;
}
});
toolBar.getChildren().addAll(addName, addLast, addRow, deleteRow, up, down);
toolBar.getChildren().addAll(addName, addRow, deleteRow, up, down);
tabPanel.setBottom(toolBar);
/* end UI for Entry table columns */

fileColumn = new CheckBox(Localization.lang("Show file column"));
urlColumn = new CheckBox(Localization.lang("Show URL/DOI column"));
Expand Down Expand Up @@ -244,7 +244,7 @@ public TableColumnsTab(JabRefPreferences prefs, JabRefFrame frame) {

Button helpButton = new Button("?");
helpButton.setPrefSize(20, 20);
helpButton.setOnAction(e->new HelpAction(Localization.lang("Help on special fields"),
helpButton.setOnAction(e -> new HelpAction(Localization.lang("Help on special fields"),
HelpFile.SPECIAL_FIELDS).getHelpButton().doClick());

rankingColumn = new CheckBox(Localization.lang("Show rank"));
Expand Down Expand Up @@ -280,7 +280,7 @@ public TableColumnsTab(JabRefPreferences prefs, JabRefFrame frame) {
GridPane specialTableColumnsBuilder = new GridPane();
specialTableColumnsBuilder.add(specialFieldsEnabled, 1, 1);
specialTableColumnsBuilder.add(rankingColumn, 1, 2);
specialTableColumnsBuilder.add(relevanceColumn, 1, 3);
specialTableColumnsBuilder.add(relevanceColumn, 1, 3);
specialTableColumnsBuilder.add(qualityColumn, 1, 4);
specialTableColumnsBuilder.add(priorityColumn, 1, 5);
specialTableColumnsBuilder.add(printedColumn, 1, 6);
Expand All @@ -295,13 +295,13 @@ public TableColumnsTab(JabRefPreferences prefs, JabRefFrame frame) {
specialTableColumnsBuilder.add(fileColumn, 2, 1);
specialTableColumnsBuilder.add(urlColumn, 2, 2);
final ToggleGroup preferUrlOrDoi = new ToggleGroup();
specialTableColumnsBuilder.add(preferUrl, 2 ,3);
specialTableColumnsBuilder.add(preferUrl, 2, 3);
specialTableColumnsBuilder.add(preferDoi, 2, 4);
preferUrl.setToggleGroup(preferUrlOrDoi);
preferDoi.setToggleGroup(preferUrlOrDoi);
specialTableColumnsBuilder.add(arxivColumn, 2, 5);

specialTableColumnsBuilder.add(extraFileColumns,2, 6);
specialTableColumnsBuilder.add(extraFileColumns, 2, 6);
specialTableColumnsBuilder.add(listOfFileColumnsScrollPane, 2, 10);

builder.add(specialTableColumnsBuilder, 1, 2);
Expand All @@ -315,10 +315,10 @@ public TableColumnsTab(JabRefPreferences prefs, JabRefFrame frame) {

Button buttonWidth = new Button("Update to current column widths");
buttonWidth.setPrefSize(300, 30);
buttonWidth.setOnAction(e->new UpdateWidthsAction());
buttonWidth.setOnAction(e -> new UpdateWidthsAction());
Button buttonOrder = new Button("Update to current column order");
buttonOrder.setPrefSize(300, 30);
buttonOrder.setOnAction(e->new UpdateOrderAction());
buttonOrder.setOnAction(e -> new UpdateOrderAction());
builder.add(buttonWidth, 1, 6);
builder.add(buttonOrder, 1, 7);
}
Expand Down Expand Up @@ -354,7 +354,7 @@ public void setValues() {
listOfFileColumns.getSelectionModel().select(indicesToSelect[i]);
}
} else {
listOfFileColumns.getSelectionModel().select(new int[] {});
listOfFileColumns.getSelectionModel().select(new int[]{});
}

/*** begin: special fields ***/
Expand Down Expand Up @@ -392,13 +392,8 @@ public void setValues() {

tableRows.clear();
List<String> names = prefs.getStringList(JabRefPreferences.COLUMN_NAMES);
List<String> lengths = prefs.getStringList(JabRefPreferences.COLUMN_WIDTHS);
for (int i = 0; i < names.size(); i++) {
if (i < lengths.size()) {
tableRows.add(new TableRow(names.get(i), Double.parseDouble(lengths.get(i))));
} else {
tableRows.add(new TableRow(names.get(i)));
}
tableRows.add(new TableRow(names.get(i)));
}
}

Expand Down Expand Up @@ -526,7 +521,6 @@ public void actionPerformed(ActionEvent e) {
/**
* Store changes to table preferences. This method is called when
* the user clicks Ok.
*
*/
@Override
public void storeSettings() {
Expand Down Expand Up @@ -574,7 +568,7 @@ public void storeSettings() {
}

// restart required implies that the settings have been changed
// the seetings need to be stored
// the settings need to be stored
if (restartRequired) {
prefs.putBoolean(JabRefPreferences.SPECIALFIELDSENABLED, newSpecialFieldsEnabled);
prefs.putBoolean(JabRefPreferences.SHOWCOLUMN_RANKING, newRankingColumn);
Expand Down
2 changes: 0 additions & 2 deletions src/main/resources/l10n/JabRef_da.properties
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,6 @@ Close\ window=Luk vindue

Closed\ library=Lukkede library

Column\ width=Kolonnebredde

Command\ line\ id=Kommandolinje-id

Contained\ in=Indeholdt i
Expand Down
2 changes: 0 additions & 2 deletions src/main/resources/l10n/JabRef_de.properties
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,6 @@ Close\ window=Fenster schließen

Closed\ library=Bibliothek geschlossen

Column\ width=Spaltenbreite

Command\ line\ id=Kommandozeilen ID
Comments=Kommentare

Expand Down
2 changes: 0 additions & 2 deletions src/main/resources/l10n/JabRef_el.properties
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,6 @@ Close\ window=Κλείσιμο παραθύρου

Closed\ library=Κλειστή βιβλιοθήκη

Column\ width=Μήκος στήλης

Command\ line\ id=id γραμμής εντολών
Comments=Σχόλια

Expand Down
2 changes: 0 additions & 2 deletions src/main/resources/l10n/JabRef_en.properties
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,6 @@ Close\ window=Close window

Closed\ library=Closed library

Column\ width=Column width

Command\ line\ id=Command line id
Comments=Comments

Expand Down
2 changes: 0 additions & 2 deletions src/main/resources/l10n/JabRef_es.properties
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,6 @@ Close\ window=Cerrar ventana

Closed\ library=Biblioteca cerrada

Column\ width=Ancho de columna

Command\ line\ id=Id de línea de comando
Comments=Comentarios

Expand Down
2 changes: 0 additions & 2 deletions src/main/resources/l10n/JabRef_fr.properties
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,6 @@ Close\ window=Fermer la fenêtre

Closed\ library=Fichier fermé

Column\ width=Largeur de colonne

Command\ line\ id=Identifiant de la ligne de commande
Comments=Commentaires

Expand Down
2 changes: 0 additions & 2 deletions src/main/resources/l10n/JabRef_in.properties
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,6 @@ Close\ window=Tutup jendela

Closed\ library=Basisdata ditutup

Column\ width=Lebar kolom

Command\ line\ id=id perintah baris

Contained\ in=Terkandung di
Expand Down
2 changes: 0 additions & 2 deletions src/main/resources/l10n/JabRef_it.properties
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,6 @@ Close\ window=Chiudi la finestra

Closed\ library=Libreria chiusa

Column\ width=Larghezza della colonna

Command\ line\ id=Identificativo della riga di comando
Comments=Commenti

Expand Down
Loading