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

Name field now in focus whenever 'add subgroup' button selected #6328

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
12 changes: 12 additions & 0 deletions .vscode/tasks.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
// See https://go.microsoft.com/fwlink/?LinkId=733558
// for the documentation about the tasks.json format
"version": "2.0.0",
"tasks": [
{
"label": "echo",
"type": "shell",
"command": "gradle"
}
]
}
Copy link
Member

Choose a reason for hiding this comment

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

This seems off-topic for this PR. Please remove the file tasks.json in the PR.

10 changes: 7 additions & 3 deletions src/main/java/org/jabref/gui/groups/GroupDialogView.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
public class GroupDialogView extends BaseDialog<AbstractGroup> {

// Basic Settings
@FXML private TextField nameField;
@FXML private TextField nameField;
Copy link
Member

Choose a reason for hiding this comment

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

Identation is off. Please restore.

In IntelliJ you have a command to autoformat code according to our standards, by selecting the code and pressing Ctrl + Alt + L. I don't know if vscode does have this option too. Maybe it's worth a few minutes researching. Saves a lot of time.

About our codingstandards, we use checkstyle to keep our standards coherent. You can install this for vscode here: https://marketplace.visualstudio.com/items?itemName=shengchen.vscode-checkstyle
I did not use checkstyle with vscode, so I'm no help in configuring it, but it should be straightforward with the documentation of the plugin.

@FXML private TextField descriptionField;
@FXML private TextField iconField;
@FXML private ColorPicker colorField;
Expand Down Expand Up @@ -90,14 +90,15 @@ public GroupDialogView(DialogService dialogService, BibDatabaseContext currentDa

@FXML
public void initialize() {
System.out.println("Group Dialog View initiated");
Copy link
Member

Choose a reason for hiding this comment

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

In JabRef, we are using a different kind of infrastructure to log messages:

private static final Logger LOGGER = LoggerFactory.getLogger(xyz.class);
[...]
LOGGER.debug("My debug message", ex);

But writing a debug message for every dialog, which is opened is not required, so this can be omitted.

hierarchyText.put(GroupHierarchyType.INCLUDING, Localization.lang("Union"));
hierarchyToolTip.put(GroupHierarchyType.INCLUDING, Localization.lang("Include subgroups: When selected, view entries contained in this group or its subgroups"));
hierarchyText.put(GroupHierarchyType.REFINING, Localization.lang("Intersection"));
hierarchyToolTip.put(GroupHierarchyType.REFINING, Localization.lang("Refine supergroup: When selected, view entries contained in both this group and its supergroup"));
hierarchyText.put(GroupHierarchyType.INDEPENDENT, Localization.lang("Independent"));
hierarchyToolTip.put(GroupHierarchyType.INDEPENDENT, Localization.lang("Independent group: When selected, view only this group's entries"));

nameField.textProperty().bindBidirectional(viewModel.nameProperty());
// nameField.textProperty().bindBidirectional(viewModel.nameProperty());
Copy link
Member

Choose a reason for hiding this comment

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

Disabling the binding of this field to the ViewModel would make the user unable to change the name of a group. Please restore the binding.

descriptionField.textProperty().bindBidirectional(viewModel.descriptionProperty());
iconField.textProperty().bindBidirectional(viewModel.iconProperty());
colorField.valueProperty().bindBidirectional(viewModel.colorFieldProperty());
Expand Down Expand Up @@ -131,7 +132,7 @@ public void initialize() {
autoGroupPersonsField.textProperty().bindBidirectional(viewModel.autoGroupPersonsFieldProperty());

texGroupFilePath.textProperty().bindBidirectional(viewModel.texGroupFilePathProperty());

validationVisualizer.setDecoration(new IconValidationDecorator());
Platform.runLater(() -> {
validationVisualizer.initVisualization(viewModel.nameValidationStatus(), nameField);
Expand All @@ -142,6 +143,7 @@ public void initialize() {
validationVisualizer.initVisualization(viewModel.keywordRegexValidationStatus(), keywordGroupSearchTerm);
validationVisualizer.initVisualization(viewModel.keywordSearchTermEmptyValidationStatus(), keywordGroupSearchTerm);
validationVisualizer.initVisualization(viewModel.keywordFieldEmptyValidationStatus(), keywordGroupSearchField);
nameField.requestFocus();
});

// Binding to the button throws a NPE, since it doesn't exist yet. Working around.
Expand All @@ -152,6 +154,8 @@ public void initialize() {
getDialogPane().lookupButton(ButtonType.OK).setDisable(true);
}
});

// nameField.requestFocus();
Copy link
Member

Choose a reason for hiding this comment

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

You can just delete this comment, as you have moved it into the Platform.runLater-block.

}

@FXML
Expand Down