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 some fields to TextArea #11886

Merged
merged 13 commits into from
Oct 13, 2024
Merged
Show file tree
Hide file tree
Changes from 11 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 @@ -87,6 +87,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
- We fixed an issue where the Undo/Redo buttons were active even when all libraries are closed. [#11837](https://github.com/JabRef/jabref/issues/11837)
- We fixed an issue where recently opened files were not displayed in the main menu properly. [#9042](https://github.com/JabRef/jabref/issues/9042)
- We fixed an issue where the DOI lookup would show an error when a DOI was found for an entry. [#11850](https://github.com/JabRef/jabref/issues/11850)
- We fixed an issue where tab cannot be used to jump to next field in some single-line fields. [#11785](https://github.com/JabRef/jabref/issues/11785)

### Removed

Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/jabref/gui/edit/CopyDoiUrlAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import java.util.Optional;

import javafx.scene.control.TextArea;
import javafx.scene.control.TextField;

import org.jabref.gui.ClipBoardManager;
import org.jabref.gui.DialogService;
Expand All @@ -16,12 +16,12 @@
*/
public class CopyDoiUrlAction extends SimpleCommand {

private final TextArea component;
private final TextField component;
private final StandardActions action;
private final DialogService dialogService;
private final ClipBoardManager clipBoardManager;

public CopyDoiUrlAction(TextArea component, StandardActions action, DialogService dialogService, ClipBoardManager clipBoardManager) {
public CopyDoiUrlAction(TextField component, StandardActions action, DialogService dialogService, ClipBoardManager clipBoardManager) {
this.component = component;
this.action = action;
this.dialogService = dialogService;
Expand Down
23 changes: 23 additions & 0 deletions src/main/java/org/jabref/gui/fieldeditors/EditorTextField.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.net.URL;
import java.util.List;
import java.util.Objects;
import java.util.ResourceBundle;
import java.util.function.Supplier;

Expand All @@ -19,6 +20,9 @@
public class EditorTextField extends TextField implements Initializable, ContextMenuAddable {

private final ContextMenu contextMenu = new ContextMenu();
private PasteActionHandler additionalPasteActionHandler = () -> {
// No additional paste behavior
};
Copy link
Member

Choose a reason for hiding this comment

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

You don't need to create an interface; you can use java.lang.Runnable instead. Please also update in org.jabref.gui.fieldeditors.EditorTextArea, or remove it if it's not being used.

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 think using Runnable here would be semantically inappropriate, since it's typically used for thread-related tasks. Sticking with a more specific interface or lambda for paste handling keeps the intent clearer.

Copy link
Member

Choose a reason for hiding this comment

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

About Runnables. I also thought it's only for threading, but no.

It's a general and accepted way to have a function type that accepts to arguments and returns no type

Copy link
Member

Choose a reason for hiding this comment

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

i think using Runnable here would be semantically inappropriate, since it's typically used for thread-related tasks. Sticking with a more specific interface or lambda for paste handling keeps the intent clearer.

Reading the documentation for Runnable, I don’t see any threading-related tasks mentioned.

The interface is similar to how you implemented PasteActionHandler. (https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/lang/Runnable.java).

Copy link
Member

Choose a reason for hiding this comment

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

This is a general code style question. Does one want to have strong typing or not. - Seeing that the setting of this is very generic, I also don't see the need for an additional class type.

() -> textField.setText(new CleanupUrlFormatter().format(new TrimWhitespaceFormatter().format(textField.getText())))

Just changing to Runnable should fix it?

Regarding the intention of runnable:

Represents an operation that does not return a result.

This is excactly the thing we are looking for here, isn't it?


Source of Runnable:

package java.lang;

/**
 * Represents an operation that does not return a result.
 *
 * <p> This is a {@linkplain java.util.function functional interface}
 * whose functional method is {@link #run()}.
 *
 * @author  Arthur van Hoff
 * @see     java.util.concurrent.Callable
 * @since   1.0
 */
@FunctionalInterface
public interface Runnable {
    /**
     * Runs this operation.
     */
    void run();
}

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 changed it to Runnable in my latest commit. Thanks.


public EditorTextField() {
this("");
Expand Down Expand Up @@ -47,4 +51,23 @@ public void initContextMenu(final Supplier<List<MenuItem>> items, KeyBindingRepo
public void initialize(URL location, ResourceBundle resources) {
// not needed
}

public void setAdditionalPasteActionHandler(PasteActionHandler handler) {
Objects.requireNonNull(handler);
this.additionalPasteActionHandler = handler;
}

@Override
public void paste() {
super.paste();
additionalPasteActionHandler.handle();
}

/**
* Interface presents user-described paste behaviour applying to paste method
*/
@FunctionalInterface
public interface PasteActionHandler {
void handle();
}
}
4 changes: 2 additions & 2 deletions src/main/java/org/jabref/gui/fieldeditors/ISSNEditor.fxml
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
<?import javafx.scene.control.Button?>
<?import javafx.scene.control.Tooltip?>
<?import javafx.scene.layout.HBox?>
<?import org.jabref.gui.fieldeditors.EditorTextArea?>
<?import org.jabref.gui.fieldeditors.EditorTextField?>
<?import org.jabref.gui.icon.JabRefIconView?>
<fx:root xmlns:fx="http://javafx.com/fxml/1" type="HBox" xmlns="http://javafx.com/javafx/8.0.112"
fx:controller="org.jabref.gui.fieldeditors.ISSNEditor">
<EditorTextArea fx:id="textArea"/>
<EditorTextField fx:id="textField"/>
<Button fx:id="journalInfoButton"
onAction="#showJournalInfo"
styleClass="icon-button">
Expand Down
10 changes: 5 additions & 5 deletions src/main/java/org/jabref/gui/fieldeditors/ISSNEditor.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@

public class ISSNEditor extends HBox implements FieldEditorFX {
@FXML private ISSNEditorViewModel viewModel;
@FXML private EditorTextArea textArea;
@FXML private EditorTextField textField;
@FXML private Button journalInfoButton;
@FXML private Button fetchInformationByIdentifierButton;

Expand Down Expand Up @@ -60,9 +60,9 @@ public ISSNEditor(Field field,
stateManager,
preferences);

establishBinding(textArea, viewModel.textProperty(), keyBindingRepository, undoAction, redoAction);
textArea.initContextMenu(new DefaultMenu(textArea), keyBindingRepository);
new EditorValidator(preferences).configureValidation(viewModel.getFieldValidator().getValidationStatus(), textArea);
establishBinding(textField, viewModel.textProperty(), keyBindingRepository, undoAction, redoAction);
textField.initContextMenu(new DefaultMenu(textField), keyBindingRepository);
new EditorValidator(preferences).configureValidation(viewModel.getFieldValidator().getValidationStatus(), textField);
}

public ISSNEditorViewModel getViewModel() {
Expand All @@ -82,7 +82,7 @@ public Parent getNode() {

@Override
public void requestFocus() {
textArea.requestFocus();
textField.requestFocus();
}

@FXML
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/jabref/gui/fieldeditors/OwnerEditor.fxml
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
<?import javafx.scene.control.Button?>
<?import javafx.scene.control.Tooltip?>
<?import javafx.scene.layout.HBox?>
<?import org.jabref.gui.fieldeditors.EditorTextArea?>
<?import org.jabref.gui.fieldeditors.EditorTextField?>
<?import org.jabref.gui.icon.JabRefIconView?>
<fx:root xmlns:fx="http://javafx.com/fxml/1" type="HBox" xmlns="http://javafx.com/javafx/8.0.112"
fx:controller="org.jabref.gui.fieldeditors.OwnerEditor">
<EditorTextArea fx:id="textArea" prefHeight="0.0" HBox.hgrow="ALWAYS"/>
<EditorTextField fx:id="textField" prefHeight="0.0" HBox.hgrow="ALWAYS"/>
<Button onAction="#setOwner"
styleClass="icon-button">
<graphic>
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/org/jabref/gui/fieldeditors/OwnerEditor.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
public class OwnerEditor extends HBox implements FieldEditorFX {

@FXML private OwnerEditorViewModel viewModel;
@FXML private EditorTextArea textArea;
@FXML private EditorTextField textField;

@Inject private GuiPreferences preferences;
@Inject private KeyBindingRepository keyBindingRepository;
Expand All @@ -38,9 +38,9 @@ public OwnerEditor(Field field,
.load();

this.viewModel = new OwnerEditorViewModel(field, suggestionProvider, preferences, fieldCheckers, undoManager);
establishBinding(textArea, viewModel.textProperty(), keyBindingRepository, undoAction, redoAction);
textArea.initContextMenu(EditorMenus.getNameMenu(textArea), keyBindingRepository);
new EditorValidator(preferences).configureValidation(viewModel.getFieldValidator().getValidationStatus(), textArea);
establishBinding(textField, viewModel.textProperty(), keyBindingRepository, undoAction, redoAction);
textField.initContextMenu(EditorMenus.getNameMenu(textField), keyBindingRepository);
new EditorValidator(preferences).configureValidation(viewModel.getFieldValidator().getValidationStatus(), textField);
}

public OwnerEditorViewModel getViewModel() {
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/jabref/gui/fieldeditors/UrlEditor.fxml
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
<?import javafx.scene.control.Button?>
<?import javafx.scene.control.Tooltip?>
<?import javafx.scene.layout.HBox?>
<?import org.jabref.gui.fieldeditors.EditorTextArea?>
<?import org.jabref.gui.fieldeditors.EditorTextField?>
<?import org.jabref.gui.icon.JabRefIconView?>
<fx:root xmlns:fx="http://javafx.com/fxml/1" type="HBox" xmlns="http://javafx.com/javafx/8.0.112"
fx:controller="org.jabref.gui.fieldeditors.UrlEditor">
<EditorTextArea fx:id="textArea" prefHeight="0.0" HBox.hgrow="ALWAYS"/>
<EditorTextField fx:id="textField" prefHeight="0.0" HBox.hgrow="ALWAYS"/>
<Button disable="${controller.viewModel.validUrlIsNotPresent}" onAction="#openExternalLink"
styleClass="icon-button">
<graphic>
Expand Down
12 changes: 6 additions & 6 deletions src/main/java/org/jabref/gui/fieldeditors/UrlEditor.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
public class UrlEditor extends HBox implements FieldEditorFX {

@FXML private final UrlEditorViewModel viewModel;
@FXML private EditorTextArea textArea;
@FXML private EditorTextField textField;

@Inject private DialogService dialogService;
@Inject private GuiPreferences preferences;
Expand All @@ -48,15 +48,15 @@ public UrlEditor(Field field,

this.viewModel = new UrlEditorViewModel(field, suggestionProvider, dialogService, preferences, fieldCheckers, undoManager);

establishBinding(textArea, viewModel.textProperty(), keyBindingRepository, undoAction, redoAction);
establishBinding(textField, viewModel.textProperty(), keyBindingRepository, undoAction, redoAction);

Supplier<List<MenuItem>> contextMenuSupplier = EditorMenus.getCleanupUrlMenu(textArea);
textArea.initContextMenu(contextMenuSupplier, preferences.getKeyBindingRepository());
Supplier<List<MenuItem>> contextMenuSupplier = EditorMenus.getCleanupUrlMenu(textField);
textField.initContextMenu(contextMenuSupplier, preferences.getKeyBindingRepository());

// init paste handler for UrlEditor to format pasted url link in textArea
textArea.setPasteActionHandler(() -> textArea.setText(new CleanupUrlFormatter().format(new TrimWhitespaceFormatter().format(textArea.getText()))));
textField.setAdditionalPasteActionHandler(() -> textField.setText(new CleanupUrlFormatter().format(new TrimWhitespaceFormatter().format(textField.getText()))));

new EditorValidator(preferences).configureValidation(viewModel.getFieldValidator().getValidationStatus(), textArea);
new EditorValidator(preferences).configureValidation(viewModel.getFieldValidator().getValidationStatus(), textField);
}

public UrlEditorViewModel getViewModel() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import javafx.scene.control.MenuItem;
import javafx.scene.control.SeparatorMenuItem;
import javafx.scene.control.TextArea;
import javafx.scene.control.TextField;
import javafx.scene.control.TextInputControl;

import org.jabref.gui.ClipBoardManager;
Expand Down Expand Up @@ -51,35 +51,35 @@ public static Supplier<List<MenuItem>> getNameMenu(final TextInputControl textIn
/**
* The default context menu with a specific menu copying a DOI/ DOI URL.
*
* @param textArea text-area that this menu will be connected to
* @param textField text-field that this menu will be connected to
* @return menu containing items of the default menu and an item for copying a DOI/DOI URL
*/
public static Supplier<List<MenuItem>> getDOIMenu(TextArea textArea, DialogService dialogService) {
public static Supplier<List<MenuItem>> getDOIMenu(TextField textField, DialogService dialogService) {
return () -> {
ActionFactory factory = new ActionFactory();
ClipBoardManager clipBoardManager = Injector.instantiateModelOrService(ClipBoardManager.class);
MenuItem copyDoiMenuItem = factory.createMenuItem(StandardActions.COPY_DOI, new CopyDoiUrlAction(textArea, StandardActions.COPY_DOI, dialogService, clipBoardManager));
MenuItem copyDoiUrlMenuItem = factory.createMenuItem(StandardActions.COPY_DOI_URL, new CopyDoiUrlAction(textArea, StandardActions.COPY_DOI_URL, dialogService, clipBoardManager));
MenuItem copyDoiMenuItem = factory.createMenuItem(StandardActions.COPY_DOI, new CopyDoiUrlAction(textField, StandardActions.COPY_DOI, dialogService, clipBoardManager));
MenuItem copyDoiUrlMenuItem = factory.createMenuItem(StandardActions.COPY_DOI_URL, new CopyDoiUrlAction(textField, StandardActions.COPY_DOI_URL, dialogService, clipBoardManager));
List<MenuItem> menuItems = new ArrayList<>();
menuItems.add(copyDoiMenuItem);
menuItems.add(copyDoiUrlMenuItem);
menuItems.add(new SeparatorMenuItem());
menuItems.addAll(new DefaultMenu(textArea).get());
menuItems.addAll(new DefaultMenu(textField).get());
return menuItems;
};
}

/**
* The default context menu with a specific menu item to cleanup URL.
*
* @param textArea text-area that this menu will be connected to
* @param textField text field that this menu will be connected to
* @return menu containing items of the default menu and an item to cleanup a URL
*/
public static Supplier<List<MenuItem>> getCleanupUrlMenu(TextArea textArea) {
public static Supplier<List<MenuItem>> getCleanupUrlMenu(TextField textField) {
return () -> {
MenuItem cleanupURL = new MenuItem(Localization.lang("Cleanup URL link"));
cleanupURL.setDisable(textArea.textProperty().isEmpty().get());
cleanupURL.setOnAction(event -> textArea.setText(new CleanupUrlFormatter().format(textArea.getText())));
cleanupURL.setDisable(textField.textProperty().isEmpty().get());
cleanupURL.setOnAction(event -> textField.setText(new CleanupUrlFormatter().format(textField.getText())));
List<MenuItem> menuItems = new ArrayList<>();
menuItems.add(cleanupURL);
return menuItems;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
<?import javafx.scene.control.Tooltip?>
<?import javafx.scene.layout.HBox?>
<?import javafx.scene.layout.StackPane?>
<?import org.jabref.gui.fieldeditors.EditorTextArea?>
<?import org.jabref.gui.fieldeditors.EditorTextField?>
<?import org.jabref.gui.icon.JabRefIconView?>
<fx:root xmlns:fx="http://javafx.com/fxml/1" type="HBox" xmlns="http://javafx.com/javafx/8.0.112"
fx:controller="org.jabref.gui.fieldeditors.identifier.IdentifierEditor">
<EditorTextArea fx:id="textArea" prefHeight="0.0" HBox.hgrow="ALWAYS"/>
<EditorTextField fx:id="textField" prefHeight="0.0" HBox.hgrow="ALWAYS"/>
<Button disable="${controller.viewModel.isInvalidIdentifier}" onAction="#openExternalLink"
styleClass="icon-button">
<graphic>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import org.jabref.gui.DialogService;
import org.jabref.gui.StateManager;
import org.jabref.gui.autocompleter.SuggestionProvider;
import org.jabref.gui.fieldeditors.EditorTextArea;
import org.jabref.gui.fieldeditors.EditorTextField;
import org.jabref.gui.fieldeditors.EditorValidator;
import org.jabref.gui.fieldeditors.FieldEditorFX;
import org.jabref.gui.fieldeditors.contextmenu.DefaultMenu;
Expand All @@ -36,7 +36,7 @@
public class IdentifierEditor extends HBox implements FieldEditorFX {

@FXML private BaseIdentifierEditorViewModel<?> viewModel;
@FXML private EditorTextArea textArea;
@FXML private EditorTextField textField;
@FXML private Button fetchInformationByIdentifierButton;
@FXML private Button lookupIdentifierButton;

Expand Down Expand Up @@ -74,20 +74,20 @@ public IdentifierEditor(Field field,
.root(this)
.load();

textArea.textProperty().bindBidirectional(viewModel.textProperty());
textField.textProperty().bindBidirectional(viewModel.textProperty());

fetchInformationByIdentifierButton.setTooltip(
new Tooltip(Localization.lang("Get bibliographic data from %0", field.getDisplayName())));
lookupIdentifierButton.setTooltip(
new Tooltip(Localization.lang("Look up %0", field.getDisplayName())));

if (field.equals(DOI)) {
textArea.initContextMenu(EditorMenus.getDOIMenu(textArea, dialogService), preferences.getKeyBindingRepository());
textField.initContextMenu(EditorMenus.getDOIMenu(textField, dialogService), preferences.getKeyBindingRepository());
} else {
textArea.initContextMenu(new DefaultMenu(textArea), preferences.getKeyBindingRepository());
textField.initContextMenu(new DefaultMenu(textField), preferences.getKeyBindingRepository());
}

new EditorValidator(preferences).configureValidation(viewModel.getFieldValidator().getValidationStatus(), textArea);
new EditorValidator(preferences).configureValidation(viewModel.getFieldValidator().getValidationStatus(), textField);
}

public BaseIdentifierEditorViewModel<?> getViewModel() {
Expand Down
Loading