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

Convert Strings Dialog to javafx #3735

Merged
merged 38 commits into from
Feb 17, 2019
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
cf7a48f
Convert String dialog to javafx
Siedlerchr Feb 17, 2018
5b15f26
add save and cancel buttons
Siedlerchr Feb 17, 2018
017ac37
add method for saving
Siedlerchr Feb 18, 2018
0d40b60
Merge branch 'maintable-beta' into editStringsjavafx
Siedlerchr Feb 22, 2018
4482a7a
Merge branch 'maintable-beta' into editStringsjavafx
Siedlerchr Feb 25, 2018
4ed242b
First attempt at validation
Siedlerchr Feb 25, 2018
e764d04
use mvvmfx validation
Siedlerchr Feb 25, 2018
f5e2a27
Add validation in factory
Siedlerchr Feb 26, 2018
f1e4cd8
Merge remote-tracking branch 'upstream/maintable-beta' into editStrin…
Siedlerchr Feb 27, 2018
aacd4f9
remove old comment
Siedlerchr Feb 27, 2018
ed77f62
add css but still does not work :(
Siedlerchr Feb 27, 2018
5d0755c
Merge remote-tracking branch 'upstream/maintable-beta' into editStrin…
Siedlerchr Apr 1, 2018
424d463
convert to new javafx view/controller
Siedlerchr Apr 1, 2018
dd63df4
Merge remote-tracking branch 'upstream/maintable-beta' into editStrin…
Siedlerchr Apr 3, 2018
84c0aba
play around with validation
Siedlerchr Apr 3, 2018
3fa8648
Merge remote-tracking branch 'upstream/master' into editStringsjavafx
Siedlerchr Jan 20, 2019
2482bd0
refactor
Siedlerchr Jan 20, 2019
db5debd
fix l10n
Siedlerchr Jan 20, 2019
ca2dc2c
Merge remote-tracking branch 'upstream/master' into editStringsjavafx
Siedlerchr Jan 21, 2019
4101f76
fix loading of string dialog
Siedlerchr Jan 21, 2019
014dd12
fix imports
Siedlerchr Jan 21, 2019
c273ecb
Merge remote-tracking branch 'upstream/master' into editStringsjavafx
Siedlerchr Feb 1, 2019
f763eaf
remove old stuff
Siedlerchr Feb 1, 2019
d973630
fix Validation
Siedlerchr Feb 2, 2019
72bd1ad
fix codacy, improve layout
Siedlerchr Feb 2, 2019
23be3e3
Refactorings and renamings
Siedlerchr Feb 3, 2019
74db19a
first part for factory
Siedlerchr Feb 3, 2019
2f15c24
Merge remote-tracking branch 'upstream/master' into editStringsjavafx
Siedlerchr Feb 4, 2019
30d5bf1
fix visibilty error
Siedlerchr Feb 4, 2019
634bed4
Merge remote-tracking branch 'upstream/master' into editStringsjavafx
Siedlerchr Feb 15, 2019
2363c15
move validation textfieldtablecell to factory
Siedlerchr Feb 15, 2019
1c2ffa3
fix import order
Siedlerchr Feb 15, 2019
49dfd2f
fix merge erros in l10n
Siedlerchr Feb 15, 2019
8ed811b
remove obsolete key from l10n
Siedlerchr Feb 15, 2019
018a432
remove unused TextField
Siedlerchr Feb 16, 2019
f5a399f
renamings and remove uncessary methods
Siedlerchr Feb 16, 2019
616cbeb
extract string adding to bibdatabase
Siedlerchr Feb 17, 2019
0082b13
fix checkstyle
Siedlerchr Feb 17, 2019
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
5 changes: 4 additions & 1 deletion src/main/java/org/jabref/gui/JabRefFrame.java
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@
import org.jabref.gui.push.PushToApplications;
import org.jabref.gui.search.GlobalSearchBar;
import org.jabref.gui.specialfields.SpecialFieldValueViewModel;
import org.jabref.gui.strings.StringAction;
import org.jabref.gui.undo.CountingUndoManager;
import org.jabref.gui.util.DefaultTaskExecutor;
import org.jabref.logic.autosaveandbackup.AutosaveManager;
Expand Down Expand Up @@ -873,13 +874,15 @@ private MenuBar createMenu() {

new SeparatorMenuItem(),


factory.createMenuItem(StandardActions.DELETE_ENTRY, new EditAction(Actions.DELETE)),

new SeparatorMenuItem(),

factory.createMenuItem(StandardActions.LIBRARY_PROPERTIES, new LibraryPropertiesAction(this)),
factory.createMenuItem(StandardActions.EDIT_PREAMBLE, new OldDatabaseCommandWrapper(Actions.EDIT_PREAMBLE, this, Globals.stateManager)),
factory.createMenuItem(StandardActions.EDIT_STRINGS, new OldDatabaseCommandWrapper(Actions.EDIT_STRINGS, this, Globals.stateManager))

factory.createMenuItem(StandardActions.EDIT_STRINGS, new StringAction())

);

Expand Down
12 changes: 12 additions & 0 deletions src/main/java/org/jabref/gui/strings/StringAction.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package org.jabref.gui.strings;
Siedlerchr marked this conversation as resolved.
Show resolved Hide resolved

import org.jabref.gui.actions.SimpleCommand;

public class StringAction extends SimpleCommand {

@Override
public void execute() {
new StringDialogView().show();
}

}
133 changes: 133 additions & 0 deletions src/main/java/org/jabref/gui/strings/StringDialogController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
package org.jabref.gui.strings;

import javax.inject.Inject;

import javafx.event.ActionEvent;
import javafx.fxml.FXML;
import javafx.scene.control.Button;
import javafx.scene.control.TableCell;
import javafx.scene.control.TableColumn;
import javafx.scene.control.TableColumn.CellEditEvent;
import javafx.scene.control.TableView;
import javafx.scene.control.Tooltip;
import javafx.scene.control.cell.TextFieldTableCell;
import javafx.util.Callback;
import javafx.util.converter.DefaultStringConverter;

import org.jabref.gui.AbstractController;
import org.jabref.gui.IconTheme.JabRefIcons;
import org.jabref.gui.StateManager;
import org.jabref.gui.help.HelpAction;
import org.jabref.logic.help.HelpFile;
import org.jabref.logic.l10n.Localization;

import org.controlsfx.tools.ValueExtractor;
import org.controlsfx.validation.ValidationSupport;
import org.controlsfx.validation.Validator;

public class StringDialogController extends AbstractController<StringDialogViewModel> {

@FXML private Button btnNewString;
@FXML private Button btnRemove;
@FXML private Button btnHelp;
@FXML private Button btnCancel;
@FXML private Button btnSave;
@FXML private TableView<StringViewModel> tblStrings;
@FXML private TableColumn<StringViewModel, String> colLabel;
@FXML private TableColumn<StringViewModel, String> colContent;
@Inject private StateManager stateManager;

@FXML
private void initialize() {

btnHelp.setGraphic(JabRefIcons.HELP.getGraphicNode());
btnHelp.setTooltip(new Tooltip(Localization.lang("Open Help page")));

btnNewString.setGraphic(JabRefIcons.ADD.getGraphicNode());
btnNewString.setTooltip(new Tooltip(Localization.lang("New string")));

btnRemove.setGraphic(JabRefIcons.REMOVE.getGraphicNode());
btnRemove.setTooltip(new Tooltip(Localization.lang("Remove string")));

viewModel = new StringDialogViewModel(stateManager);
colLabel.setCellValueFactory(cellData -> cellData.getValue().getLabel());
colContent.setCellValueFactory(cellData -> cellData.getValue().getContent());

ValidationSupport validationSupport = new ValidationSupport();

//Register TextfieldTableCell Control for validation
ValueExtractor.addObservableValueExtractor(c -> c instanceof TextFieldTableCell, c -> ((TextFieldTableCell<?, String>) c).textProperty());

colLabel.setCellFactory(new Callback<TableColumn<StringViewModel, String>, TableCell<StringViewModel, String>>() {

@Override
public TableCell<StringViewModel, String> call(TableColumn<StringViewModel, String> param) {

TextFieldTableCell<StringViewModel, String> cell = new TextFieldTableCell<>(new DefaultStringConverter());
validationSupport.registerValidator(cell, Validator.createEmptyValidator("Text is required"));
return cell;
}

});

colContent.setCellFactory(new Callback<TableColumn<StringViewModel, String>, TableCell<StringViewModel, String>>() {

@Override
public TableCell<StringViewModel, String> call(TableColumn<StringViewModel, String> param) {

TextFieldTableCell<StringViewModel, String> cell = new TextFieldTableCell<>(new DefaultStringConverter());
validationSupport.registerValidator(cell, Validator.createEmptyValidator("Text is required"));
return cell;
}
});

colLabel.setOnEditCommit(
(CellEditEvent<StringViewModel, String> t) -> {
t.getTableView()
.getItems()
.get(
t.getTablePosition().getRow())
Copy link
Member

Choose a reason for hiding this comment

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

t.getRowValue is easier (and please rename t to something more descriptive.

.setLabel(t.getNewValue());
});
colContent.setOnEditCommit(
(CellEditEvent<StringViewModel, String> t) -> {
t.getTableView()
.getItems()
.get(
t.getTablePosition().getRow())
.setContent(t.getNewValue());
});

tblStrings.itemsProperty().bindBidirectional(viewModel.allStringsProperty());
tblStrings.setEditable(true);

tblStrings.getSelectionModel().selectedItemProperty().addListener(
(observable, oldValue, newValue) -> viewModel.validateInput(newValue));
}

@FXML
private void addString(ActionEvent event) {
viewModel.addNewString();
}

@FXML
private void openHelp(ActionEvent event) {
HelpAction.openHelpPage(HelpFile.STRING_EDITOR);
}

@FXML
private void removeString(ActionEvent event) {
StringViewModel selected = tblStrings.getSelectionModel().getSelectedItem();
viewModel.removeString(selected);
}

@FXML
private void save() {
viewModel.save();
}

@FXML
private void close() {
getStage().close();
}
}
18 changes: 18 additions & 0 deletions src/main/java/org/jabref/gui/strings/StringDialogView.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package org.jabref.gui.strings;

import javafx.scene.control.Alert.AlertType;
import javafx.scene.control.DialogPane;

import org.jabref.gui.AbstractDialogView;
import org.jabref.gui.FXDialog;

public class StringDialogView extends AbstractDialogView {

@Override
public void show() {
FXDialog stringDialog = new FXDialog(AlertType.INFORMATION, "Edit Strings");
stringDialog.setDialogPane((DialogPane) this.getView());
stringDialog.setResizable(true);
stringDialog.show();
}
}
84 changes: 84 additions & 0 deletions src/main/java/org/jabref/gui/strings/StringDialogViewModel.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
package org.jabref.gui.strings;

import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;

import javafx.beans.property.ListProperty;
import javafx.beans.property.SimpleListProperty;
import javafx.collections.FXCollections;

import org.jabref.gui.AbstractViewModel;
import org.jabref.gui.StateManager;
import org.jabref.logic.bibtex.comparator.BibtexStringComparator;
import org.jabref.model.database.BibDatabase;
import org.jabref.model.entry.BibtexString;

public class StringDialogViewModel extends AbstractViewModel {

private final ListProperty<StringViewModel> allStrings = new SimpleListProperty<>(FXCollections.observableArrayList());

private final BibDatabase db;
Copy link
Member

Choose a reason for hiding this comment

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

no abbreviation please


public StringDialogViewModel(StateManager stateManager) {
this.db = stateManager.getActiveDatabase().get().getDatabase();
addAllStringsFromDB();

}

private void addAllStringsFromDB() {

Copy link
Member

Choose a reason for hiding this comment

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

remove empty line

Set<StringViewModel> strings = db.getStringKeySet()
.stream()
.map(string -> db.getString(string))
.sorted(new BibtexStringComparator(false))
.map(this::convertFromBibTexString)
.collect(Collectors.toSet());
allStrings.addAll(strings);
}

public ListProperty<StringViewModel> allStringsProperty() {
return this.allStrings;
}

public void addNewString() {
allStrings.add(new StringViewModel("new Label", "New Content"));
}

public void removeString(StringViewModel selected) {
allStrings.remove(selected);

}

private StringViewModel convertFromBibTexString(BibtexString bibtexString) {
Copy link
Member

Choose a reason for hiding this comment

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

move to StringViewModel or even better just create a constructor that admits a BibtexString.

return new StringViewModel(bibtexString.getName(), bibtexString.getContent());

}

public void validateInput(StringViewModel newValue) {
if (newValue != null) {
String label = newValue.getLabel().getValue();
String content = newValue.getContent().getValue();

//TODO: Validation
}

}

public void save() {
for (StringViewModel model : allStrings) {
Copy link
Member

Choose a reason for hiding this comment

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

move this to a new method BibDatabase.setStrings(Collection<BibtexString>)) ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not that easy, it's a HashMap which explicitly checks the key before inserting and generates a new id.

/**
   * Inserts a Bibtex String.
   */
  public synchronized void addString(BibtexString string) throws KeyCollisionException {
      if (hasStringLabel(string.getName())) {
          throw new KeyCollisionException("A string with that label already exists");
      }

      if (bibtexStrings.containsKey(string.getId())) {
          throw new KeyCollisionException("Duplicate BibTeX string id.");
      }

      bibtexStrings.put(string.getId(), string);
  }


Copy link
Member

Choose a reason for hiding this comment

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

I meant you should simply refactor the code and move (most) of it to a new method in BibDatabase (not re-implement it in another way).

String label = model.getLabel().getValue();
String content = model.getContent().getValue();

Optional<BibtexString> bibtexString = db.getStringByName(label);
if (bibtexString.isPresent()) {
if (!(bibtexString.get().getContent().equals(content))) {
bibtexString.get().setContent(content);
}
} else {
db.addString(new BibtexString(label, content));
}
}

}
}
33 changes: 33 additions & 0 deletions src/main/java/org/jabref/gui/strings/StringViewModel.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package org.jabref.gui.strings;

import javafx.beans.property.SimpleStringProperty;
import javafx.beans.property.StringProperty;


public class StringViewModel {

private final StringProperty label;
private final StringProperty content;

public StringViewModel(String label, String content) {

Siedlerchr marked this conversation as resolved.
Show resolved Hide resolved
this.label = new SimpleStringProperty(label);
this.content = new SimpleStringProperty(content);
}

public StringProperty getLabel() {
return label;
}

public StringProperty getContent() {
return content;
}

public void setLabel(String label) {
this.label.setValue(label);
}

public void setContent(String content) {
this.content.setValue(content);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,17 @@ public IconValidationDecorator(Pos position) {
this.position = position;
}

@Override
protected Node createErrorNode() {
return IconTheme.JabRefIcons.ERROR.getGraphicNode();
}

@Override
protected Node createWarningNode() {
return IconTheme.JabRefIcons.WARNING.getGraphicNode();
}

@Override
public Node createDecorationNode(ValidationMessage message) {
Node graphic = Severity.ERROR == message.getSeverity() ? createErrorNode() : createWarningNode();
graphic.getStyleClass().add(Severity.ERROR == message.getSeverity() ? "error-icon" : "warning-icon");
Expand All @@ -49,6 +52,7 @@ public Node createDecorationNode(ValidationMessage message) {
return label;
}

@Override
protected Tooltip createTooltip(ValidationMessage message) {
Tooltip tooltip = new Tooltip(message.getText());
tooltip.getStyleClass().add(Severity.ERROR == message.getSeverity() ? "tooltip-error" : "tooltip-warning");
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/model/database/BibDatabase.java
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ public BibtexString getString(String id) {
}

/**
* Returns the string with the given name.
* Returns the string with the given name/label
*/
public Optional<BibtexString> getStringByName(String name) {
for (BibtexString string : getStringValues()) {
Expand Down
21 changes: 15 additions & 6 deletions src/main/java/org/jabref/model/entry/BibtexString.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* This class models a BibTex String ("@String")
*/
public class BibtexString implements Cloneable {

/**
* Type of a \@String.
* <p>
Expand Down Expand Up @@ -95,6 +96,10 @@ public void setId(String id) {
hasChanged = true;
}

/**
* Returns the name/label of the string
* @return
*/
public String getName() {
return name;
}
Expand Down Expand Up @@ -173,15 +178,19 @@ public String toString() {

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
if (this == o) {
return true;
}
if ((o == null) || (getClass() != o.getClass())) {
return false;
}
BibtexString that = (BibtexString) o;
return hasChanged == that.hasChanged &&
return (Objects.equals(hasChanged,that.hasChanged) &&
Objects.equals(name, that.name) &&
Objects.equals(content, that.content) &&
Objects.equals(id, that.id) &&
type == that.type &&
Objects.equals(parsedSerialization, that.parsedSerialization);
Objects.equals(id, that.id) &&
Objects.equals(type, that.type) &&
Objects.equals(parsedSerialization, that.parsedSerialization));
}

@Override
Expand Down
Loading