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 order of string in editor #10083

Merged
merged 6 commits into from
Jul 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We fixed an issue where empty grey containers would remain in the groups panel, if displaying of group item count is turned off. [#9972](https://github.com/JabRef/jabref/issues/9972)
- We fixed an issue where fetching an ISBN could lead to application freezing when the fetcher did not return any results. [#9979](https://github.com/JabRef/jabref/issues/9979)
- We fixed an issue where closing a library containing groups and entries caused an exception [#9997](https://github.com/JabRef/jabref/issues/9997)
- We fixed a bug where the editor for strings in a bibliography file did not sort the entries by their keys [#10083](https://github.com/JabRef/jabref/pull/10083)

### Removed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ public void initialize() {
.install(labelColumn, new DefaultStringConverter());
labelColumn.setOnEditCommit((TableColumn.CellEditEvent<ConstantsItemModel, String> cellEvent) -> {

ConstantsItemModel cellItem = cellEvent.getTableView()
.getItems()
var tableView = cellEvent.getTableView();
ConstantsItemModel cellItem = tableView.getItems()
.get(cellEvent.getTablePosition().getRow());

Optional<ConstantsItemModel> existingItem = viewModel.labelAlreadyExists(cellEvent.getNewValue());
Expand All @@ -77,7 +77,13 @@ public void initialize() {
cellItem.setLabel(cellEvent.getNewValue());
}

cellEvent.getTableView().refresh();
// Resort the entries based on the keys and set the focus to the newly-created entry
viewModel.resortStrings();
var selectionModel = tableView.getSelectionModel();
selectionModel.select(cellItem);
selectionModel.focus(selectionModel.getSelectedIndex());
tableView.refresh();
tableView.scrollTo(cellItem);
});

contentColumn.setSortable(true);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.jabref.gui.libraryproperties.constants;

import java.util.Comparator;
import java.util.Locale;
import java.util.Optional;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -48,7 +50,7 @@ public void setValues() {
stringsListProperty.addAll(databaseContext.getDatabase().getStringValues().stream()
.sorted(new BibtexStringComparator(false))
.map(this::convertFromBibTexString)
.collect(Collectors.toSet()));
.toList());
}

public void addNewString() {
Expand All @@ -70,6 +72,11 @@ public void removeString(ConstantsItemModel item) {
stringsListProperty.remove(item);
}

public void resortStrings() {
// Resort the strings list in the same order as setValues() does
stringsListProperty.sort(Comparator.comparing(c -> c.labelProperty().get().toLowerCase(Locale.ROOT)));
}

private ConstantsItemModel convertFromBibTexString(BibtexString bibtexString) {
return new ConstantsItemModel(bibtexString.getName(), bibtexString.getContent());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package org.jabref.gui.libraryproperties.constants;

import java.util.List;

import javafx.beans.property.StringProperty;

import org.jabref.gui.DialogService;
import org.jabref.model.database.BibDatabase;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibtexString;

import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.Mockito.mock;

class ConstantsPropertiesViewModelTest {

@DisplayName("Check that the list of strings is sorted according to their keys")
@Test
void testStringsListPropertySorting() {
BibtexString string1 = new BibtexString("TSE", "Transactions on Software Engineering");
BibtexString string2 = new BibtexString("ICSE", "International Conference on Software Engineering");
BibDatabase db = new BibDatabase();
db.setStrings(List.of(string1, string2));
BibDatabaseContext context = new BibDatabaseContext(db);
DialogService service = mock(DialogService.class);
List<String> expected = List.of(string2.getName(), string1.getName()); // ICSE before TSE

ConstantsPropertiesViewModel model = new ConstantsPropertiesViewModel(context, service);
model.setValues();

List<String> actual = model.stringsListProperty().stream()
.map(ConstantsItemModel::labelProperty)
.map(StringProperty::getValue)
.toList();

assertEquals(expected, actual);
}

@DisplayName("Check that the list of strings is sorted after resorting it")
@Test
void testStringsListPropertyResorting() {
BibDatabase db = new BibDatabase();
BibDatabaseContext context = new BibDatabaseContext(db);
DialogService service = mock(DialogService.class);
List<String> expected = List.of("ICSE", "TSE");

ConstantsPropertiesViewModel model = new ConstantsPropertiesViewModel(context, service);
var stringsList = model.stringsListProperty();
stringsList.add(new ConstantsItemModel("TSE", "Transactions on Software Engineering"));
stringsList.add(new ConstantsItemModel("ICSE", "International Conference on Software Engineering"));

model.resortStrings();

List<String> actual = model.stringsListProperty().stream()
.map(ConstantsItemModel::labelProperty)
.map(StringProperty::getValue)
.toList();

assertEquals(expected, actual);
}
}
Loading