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

Jump to bottom

Enable editing typo with double clicking on field name in custom entry type #9977

Merged
merged 15 commits into from
Jun 8, 2023
Merged

Enable editing typo with double clicking on field name in custom entry type #9977

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
b7b6b49
Merge pull request #1 from JabRef/main
DinjerChang Jun 5, 2023
ab673dc
frontend custom entry type edit feature
DinjerChang Jun 5, 2023
fc6c245
new function setName() in UnkownField.java added so we could update t…
DinjerChang Jun 6, 2023
ed7373c
CHANGELOG.MD added
DinjerChang Jun 6, 2023
8a2daa0
checkstyle fixed, better varible name modified in CustomEntryTypesTab…
DinjerChang Jun 6, 2023
d653fc5
dialog with notify functionality and use equalsIgnoreCase when compar…
DinjerChang Jun 6, 2023
2be4ccb
add Localization.lang("key") to JabRed_en.properties
DinjerChang Jun 7, 2023
b6cd9b7
Capitalizing first letter when displaying on coulumn after edit
DinjerChang Jun 7, 2023
e7d288c
fix when user input the original field name or doesnt edit anything a…
DinjerChang Jun 7, 2023
6d4e751
Merge branch 'main' into fix-for-issue-9840
DinjerChang Jun 7, 2023
574cd4e
fix wrong placement of predicate: !newFieldValue.equals(field.getDisp…
DinjerChang Jun 8, 2023
45681d9
Merge branch 'fix-for-issue-9840' of https://github.com/DinjerChang/j…
DinjerChang Jun 8, 2023
66bc727
Merge branch 'main' into fix-for-issue-9840
koppor Jun 8, 2023
ee41ca9
Update CHANGELOG.md
calixtus Jun 8, 2023
d19f84f
Fix localization
koppor Jun 8, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We fixen an issue under Linux where under some systems the file instead of the folder was openend [#9607](https://github.com/JabRef/jabref/issues/9607)
- We fixed an issue where an Automatic Keyword Group could not be deleted in the UI. [#9778](https://github.com/JabRef/jabref/issues/9778)
- We fixed an issue where the citation key pattern `[edtrN_M]` returned the wrong editor. [#9946](https://github.com/JabRef/jabref/pull/9946)
- We fixed an issue where allow user to update the value of field in custom entry type with double-clicking it. [#9840](https://github.com/JabRef/jabref/issues/9840)


### Removed
Expand Down
26 changes: 26 additions & 0 deletions src/main/java/org/jabref/gui/preferences/customentrytypes/CustomEntryTypesTab.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import javafx.application.Platform;
import javafx.beans.property.ReadOnlyStringWrapper;
import javafx.collections.ObservableList;
import javafx.fxml.FXML;
import javafx.scene.Group;
import javafx.scene.control.Button;
Expand All @@ -12,6 +13,7 @@
import javafx.scene.control.TableView;
import javafx.scene.control.TextField;
import javafx.scene.control.cell.CheckBoxTableCell;
import javafx.scene.control.cell.TextFieldTableCell;
import javafx.scene.input.ClipboardContent;
import javafx.scene.input.DragEvent;
import javafx.scene.input.Dragboard;
Expand All @@ -33,6 +35,7 @@
import org.jabref.model.database.BibDatabaseMode;
import org.jabref.model.entry.BibEntryTypesManager;
import org.jabref.model.entry.field.Field;
import org.jabref.model.entry.field.UnknownField;

import com.airhacks.afterburner.views.ViewLoader;
import com.tobiasdiez.easybind.EasyBind;
Expand Down Expand Up @@ -143,6 +146,29 @@ private void setupEntryTypesTable() {

private void setupFieldsTable() {
fieldNameColumn.setCellValueFactory(item -> item.getValue().nameProperty());
fieldNameColumn.setCellFactory(TextFieldTableCell.forTableColumn());
fieldNameColumn.setEditable(true);
fieldNameColumn.setOnEditCommit(
(TableColumn.CellEditEvent<FieldViewModel, String> event) -> {
String newFieldValue = event.getNewValue();
UnknownField field = (UnknownField) event.getRowValue().getField();
EntryTypeViewModel selectedEntryType = viewModel.selectedEntryTypeProperty().get();
ObservableList<FieldViewModel> entryFields = selectedEntryType.fields();

boolean fieldExists = entryFields.stream().anyMatch(fieldViewModel ->
fieldViewModel.nameProperty().getValue().equals(newFieldValue));

if (!fieldExists) {
event.getRowValue().setField(newFieldValue);
field.setName(newFieldValue);
} else {
dialogService.showWarningDialogAndWait(
Localization.lang("Duplicate fields"),
Localization.lang("Warning: You added field \"%0\" twice. Only one will be kept.", newFieldValue));
event.getTableView().edit(-1, null);
event.getTableView().refresh();
}
Copy link
Member

Choose a reason for hiding this comment

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

Modern software seldom makes use of popups. I would use the notifiy functionality. The user has no choice. Moreover, the current code keeps the old casing. Thus, instead of newFieldValue, the existing one should be output. As a consequence, the code of fieldExists needs to be modified accordingly.

Copy link
Contributor Author

@DinjerChang DinjerChang Jun 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The reason I use newFieldValue instead of the existing one is to see if the user input value (newFieldValue) exist in the current fields. Let's say we already have two new Field, Name and Test. When a user try to edit Name to Test, he would double click the Name Field and type "Test", so "Test" is the newFieldValue. So I think we have to see what user input is and compare it with the existing Field. In this case, Test already exist, so we will notify the user.

If we choose the existing one by using the method field.getDisplayName(), the value would be Name since we haven't modify the field yet. In this case, we always get the fieldExists value to be True

Also, I'm replacing equals with equlsIgnoreCase to handle the casing comparison problem.
Let's say a user edit Name to name, it should be and will be seen as duplicate field.

Let me know if I understand statement correctly and if I'm on the right track :)

Copy link
Member

Choose a reason for hiding this comment

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

Sorry that I was not clear in my terms. Thank you for explaining a scenario. I hope, this short explanation based on your scenario helps:

I was trying to say: User renames to TEST, but Test exists. I assume the user knows, he input TEST, but forgot about the existing field Test. Therefore, Test should be displayed and not TEST.

});

fieldTypeColumn.setCellFactory(CheckBoxTableCell.forTableColumn(fieldTypeColumn));
fieldTypeColumn.setCellValueFactory(item -> item.getValue().requiredProperty());
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/org/jabref/gui/preferences/customentrytypes/FieldViewModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ public FieldPriority getPriority() {
return priorityProperty.getValue();
}

public void setField(String field) {
this.fieldName.setValue(field);
}

public BibField toBibField() {
return new BibField(field, priorityProperty.getValue());
}
Expand Down
6 changes: 5 additions & 1 deletion src/main/java/org/jabref/model/entry/field/UnknownField.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import java.util.Set;

public class UnknownField implements Field {
private final String name;
private String name;
private final Set<FieldProperty> properties;

public UnknownField(String name) {
Expand All @@ -29,6 +29,10 @@ public String getName() {
return name;
}

public void setName(String name) {
this.name = name;
}

@Override
public boolean isStandardField() {
return false;
Expand Down