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

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

Closed
wants to merge 2 commits into from
Closed

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

wants to merge 2 commits into from

Conversation

HussainArif12
Copy link
Contributor

@HussainArif12 HussainArif12 commented Apr 20, 2020 •
edited by calixtus
Loading

fixes #6307

Name Text field is now in focus whenever the 'add subgroup' button is added

  • Change in CHANGELOG.md described (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

calixtus
calixtus requested changes Apr 20, 2020
View reviewed changes
Copy link
Member

@calixtus calixtus left a comment

Choose a reason for hiding this comment

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

Hi @HussainArif12 . Congratulations and thank you very much for you very first contribution to JabRef. We are always happy to welcome new people here.

I have some small comments on your PR. Please fix them and as soon as a second core developer approves, we can merge this into the master branch.

.vscode/tasks.json Outdated
"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.

src/main/java/org/jabref/gui/groups/GroupDialogView.java Outdated
@@ -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.

src/main/java/org/jabref/gui/groups/GroupDialogView.java Outdated
@@ -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.

src/main/java/org/jabref/gui/groups/GroupDialogView.java Outdated
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.

src/main/java/org/jabref/gui/groups/GroupDialogView.java Outdated
@@ -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.

@calixtus
Copy link
Member

Also please don't forget to mention your fix in the changelog.

@calixtus
Copy link
Member

Did you close your PR by accident?

@HussainArif12
Copy link
Contributor Author

HussainArif12 commented Apr 20, 2020 via email

koppor pushed a commit that referenced this pull request Dec 15, 2022
Squashed 'buildres/csl/csl-styles/' changes from 7a10e3f..f6c778e
d257fef 
f6c778e Update emerald-harvard.csl (#6335)
d6c6a16 Fix Brazilian quotes on  chicago-author-date.csl (#6317)
a1549b6 Update medizinische-hochschule-hannover.csl (#6330)
da88073 Update journal-of-the-american-college-of-cardiology.csl (#6334)
a520d8e Bump nokogiri from 1.13.9 to 1.13.10 (#6333)
ba54b44 Update royal-society-of-chemistry.csl (#6328)
1378ba7 LUSEM: Remove full stop (#6332)
9e3cf89 Create interpreting.csl (#6254)
bef74ed Create conservation-science-and-practice.csl (#6258)
9fb7eb7 Bug fixes triangle.csl (#6251)
e6112ba Update ucl-university-college-apa.csl (#6250)
6dcba3a Update engineering-technology-and-applied-science-research.csl (#6247)
00fe4a2 Create constructivist-foundations.csl (#6243)
03ad71b Create les-mondes-du-travail.csl (#6234)
a2bce86 Corrections based on author instructions (#6306)

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: f6c778e
koppor pushed a commit that referenced this pull request Dec 17, 2022
Squashed 'buildres/csl/csl-styles/' changes from 7a10e3f..84dba23
aa76383 
84dba23 Update international-union-of-crystallography.csl (#6279)
13dd9e8 Update zeitschrift-fur-deutsche-philologie.csl (#6340)
d95b652 Create cahiers-mondes-anciens.csl (#6203)
ded567c Create rassegna-degli-archivi-di-stato-bibliografia-generale.csl (#6275)
124777a Create scientific-online-letters-on-the-atmosphere.csl (#6261)
3c276e7 Create american-medical-association-no-url-alphabetical.csl (#6252)
595ad95 Bump mathieudutour/github-tag-action from 6.0 to 6.1 (#6287)
7008128 Create cambridge-a (#6336)
17e930c Update norsk-apa-manual-note.csl (#6338)
b360859 Update norsk-apa-manual.csl (#6337)
f6c778e Update emerald-harvard.csl (#6335)
d6c6a16 Fix Brazilian quotes on  chicago-author-date.csl (#6317)
a1549b6 Update medizinische-hochschule-hannover.csl (#6330)
da88073 Update journal-of-the-american-college-of-cardiology.csl (#6334)
a520d8e Bump nokogiri from 1.13.9 to 1.13.10 (#6333)
ba54b44 Update royal-society-of-chemistry.csl (#6328)
1378ba7 LUSEM: Remove full stop (#6332)
9e3cf89 Create interpreting.csl (#6254)
bef74ed Create conservation-science-and-practice.csl (#6258)
9fb7eb7 Bug fixes triangle.csl (#6251)
e6112ba Update ucl-university-college-apa.csl (#6250)
6dcba3a Update engineering-technology-and-applied-science-research.csl (#6247)
00fe4a2 Create constructivist-foundations.csl (#6243)
03ad71b Create les-mondes-du-travail.csl (#6234)
a2bce86 Corrections based on author instructions (#6306)

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: 84dba23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@calixtus calixtus calixtus requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

"Add subgroup" dialog should not start with focus on OK button
2 participants
@HussainArif12 @calixtus