Skip to content

Commit

Permalink
improve unlinked files dialog (#12195)
Browse files Browse the repository at this point in the history
* Implement changes for issue #11878: Search Unlinked local files: Improve dialog

* test: add unit test for verifying relative path functionality

* refactor: changes based on PR feedback

* Refactor: Move ellipsis logic to specific columns to avoid shared component issues

* Refactor: Replaced screen width-based calculation with column width in createEllipsisLabel method.

* FIX: Unnecessary new line removed

* fix: Unnecessary comments removed

---------

Co-authored-by: Anna <anna.gondekova@student.tuke.sk>
  • Loading branch information
agondekova and AnickaG authored Dec 2, 2024
1 parent 0170044 commit 17c3e2c
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@
<TableColumn fx:id="colMessage" prefWidth="320.0" text="%Message"/>
</columns>
<columnResizePolicy>
<TableView fx:constant="UNCONSTRAINED_RESIZE_POLICY"/>
<TableView fx:constant="CONSTRAINED_RESIZE_POLICY"/>
</columnResizePolicy>
</TableView>
</TitledPane>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import javafx.scene.control.ContextMenu;
import javafx.scene.control.Control;
import javafx.scene.control.Label;
import javafx.scene.control.OverrunStyle;
import javafx.scene.control.ProgressIndicator;
import javafx.scene.control.SelectionMode;
import javafx.scene.control.TableColumn;
Expand Down Expand Up @@ -207,19 +208,23 @@ private void initUnlinkedFilesList() {
private void initResultTable() {
colFile.setCellValueFactory(cellData -> cellData.getValue().file());
new ValueTableCellFactory<ImportFilesResultItemViewModel, String>()
.withText(item -> item).withTooltip(item -> item)
.withGraphic(this::createEllipsisLabel)
.withTooltip(item -> item)
.install(colFile);

colMessage.setCellValueFactory(cellData -> cellData.getValue().message());
new ValueTableCellFactory<ImportFilesResultItemViewModel, String>()
.withText(item -> item).withTooltip(item -> item)
.withGraphic(this::createEllipsisLabel)
.withTooltip(item -> item)
.install(colMessage);

colStatus.setCellValueFactory(cellData -> cellData.getValue().icon());
colStatus.setCellFactory(new ValueTableCellFactory<ImportFilesResultItemViewModel, JabRefIcon>().withGraphic(JabRefIcon::getGraphicNode));
importResultTable.setColumnResizePolicy(param -> true);

colFile.setResizable(true);
colStatus.setResizable(true);
colMessage.setResizable(true);
importResultTable.setItems(viewModel.resultTableItems());
importResultTable.setColumnResizePolicy(TableView.CONSTRAINED_RESIZE_POLICY);
}

private void initButtons() {
Expand Down Expand Up @@ -271,6 +276,18 @@ void exportSelected() {
viewModel.startExport();
}

/**
* Creates a Label with a maximum width and ellipsis for overflow.
* Truncates text if it exceeds two-thirds of the screen width.
*/
private Label createEllipsisLabel(String text) {
Label label = new Label(text);
double maxWidth = colFile.getMaxWidth();
label.setMaxWidth(maxWidth);
label.setTextOverrun(OverrunStyle.LEADING_ELLIPSIS);
return label;
}

/**
* Expands or collapses the specified tree according to the <code>expand</code>-parameter.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,11 @@ public void startSearch() {
}

public void startImport() {
Path directory = this.getSearchDirectory();
List<Path> fileList = checkedFileListProperty.stream()
.map(item -> item.getValue().getPath())
.filter(path -> path.toFile().isFile())
.map(path -> directory.relativize(path))
.collect(Collectors.toList());
if (fileList.isEmpty()) {
LOGGER.warn("There are no valid files checked");
Expand Down Expand Up @@ -202,7 +204,7 @@ public void startExport() {
} catch (IOException e) {
LOGGER.error("Error exporting", e);
}
}
}

public ObservableList<FileExtensionViewModel> getFileFilters() {
return this.fileFilterList;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
package org.jabref.gui.externalfiles;

import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;

import javafx.beans.property.SimpleListProperty;
import javafx.collections.FXCollections;
import javafx.scene.control.TreeItem;

import org.jabref.gui.StateManager;
import org.jabref.gui.preferences.GuiPreferences;
import org.jabref.gui.util.FileNodeViewModel;
import org.jabref.logic.FilePreferences;
import org.jabref.logic.util.TaskExecutor;
import org.jabref.model.database.BibDatabaseContext;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;

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

public class UnlinkedFilesDialogViewModelTest {
@TempDir
Path tempDir;
@TempDir
Path subDir;
@TempDir
Path file1;
@TempDir
Path file2;
@Mock
private TaskExecutor taskExecutor;
@Mock
private GuiPreferences guiPreferences;
@Mock
private StateManager stateManager;
@Mock
private BibDatabaseContext bibDatabaseContext;


private UnlinkedFilesDialogViewModel viewModel;
@BeforeEach
public void setUp() throws Exception {
MockitoAnnotations.openMocks(this);

// Mock a base directory
FilePreferences filePreferences = mock(FilePreferences.class);
when(guiPreferences.getFilePreferences()).thenReturn(filePreferences);
when(filePreferences.getWorkingDirectory()).thenReturn(Paths.get("C:/test/base"));

// Mock the state manager to provide an active database
when(stateManager.getActiveDatabase()).thenReturn(Optional.of(bibDatabaseContext));

viewModel = new UnlinkedFilesDialogViewModel(
null,
null,
null,
guiPreferences,
stateManager,
taskExecutor
);
}

@Test
public void startImportWithValidFilesTest() throws Exception {
// Create temporary test files
tempDir = Files.createTempDirectory("testDir");
subDir = tempDir.resolve("subdir");
Files.createDirectories(subDir);

// Create test files: one in the main directory and one in the subdirectory
file1 = Files.createTempFile(tempDir, "file1", ".pdf");
file2 = Files.createTempFile(subDir, "file2", ".txt");

// Mock file nodes with the absolute paths of the temporary files
FileNodeViewModel fileNode1 = mock(FileNodeViewModel.class);
FileNodeViewModel fileNode2 = mock(FileNodeViewModel.class);

when(fileNode1.getPath()).thenReturn(file1);
when(fileNode2.getPath()).thenReturn(file2);

// Create TreeItem for each FileNodeViewModel
TreeItem<FileNodeViewModel> treeItem1 = new TreeItem<>(fileNode1);
TreeItem<FileNodeViewModel> treeItem2 = new TreeItem<>(fileNode2);

SimpleListProperty<TreeItem<FileNodeViewModel>> checkedFileListProperty =
new SimpleListProperty<>(FXCollections.observableArrayList(treeItem1, treeItem2));

assertEquals(2, checkedFileListProperty.get().size());
assertEquals(file1, checkedFileListProperty.get().getFirst().getValue().getPath());
assertEquals(file2, checkedFileListProperty.get().getLast().getValue().getPath());

Path directory = tempDir; // Base directory for relativization

// Create list of relative paths
List<Path> fileList = checkedFileListProperty.stream()
.map(item -> item.getValue().getPath())
.filter(path -> path.toFile().isFile())
.map(path -> directory.relativize(path))
.collect(Collectors.toList());
assertEquals(
List.of(directory.relativize(file1), directory.relativize(file2)),
fileList,
"fileList should contain exactly the relative paths of file1.pdf and file2.txt"
);
}
}

0 comments on commit 17c3e2c

Please sign in to comment.