Fixed group creation with default settings by harinda05 · Pull Request #4801 · JabRef/jabref

harinda05 · 2019-03-22T06:36:28Z

Fix for the issue #4783

Now the enabling/disabling the OK button with regards to the text fields have been validated properly without changing the logic of the existing code. As @Siedlerchr mentioned now, no need to switch to another radio button and back again to enable the OK button.

Change in CHANGELOG.md described
Tests created for changes
Manually tested changed features in running JabRef
Screenshots added in PR description (for bigger UI changes)
Ensured that the git commit message is a good one
Check documentation status (Issue created for outdated help page at help.jabref.org?)

…ith default settings

tobiasdiez

Thanks for your PR. The code looks good to me and seems to do the job.

Next time please have a look if somebody else is already working on the same issue before you open a PR (in this case #4799 is attacking the same problem). It's a bit unfair to other contributors if there work is rendered obsolete because somebody else opened another PR on the same topic. Thanks.

tobiasdiez · 2019-03-22T09:49:24Z

            this.setTitle(Localization.lang("Edit group"));
        }

+        nameField.textProperty().addListener((observable, oldValue, newValue) -> {


Can you please move these lines further down to https://github.com/JabRef/jabref/pull/4801/files#diff-2afbba630cb28bced7c30efa7c0a24dfR362, where similar listeners are registered for the other controls. Please also remove the onAction listener for these three controls as they are now obsolete.

I'm really sorry that I didn't notice that there's someone else working on this. I have made the necessary changes you've requested.
Regards,
Harinda

tobiasdiez · 2019-03-22T09:50:54Z

 - We fixed an issue where only one PDF file could be imported [#4422](https://github.com/JabRef/jabref/issues/4422)
 - We fixed an issue where "Move to group" would always move the first entry in the library and not the selected [#4414](https://github.com/JabRef/jabref/issues/4414)
 - We fixed an issue where an older dialog appears when downloading full texts from the quality menu. [#4489](https://github.com/JabRef/jabref/issues/4489)
+- We fixed an issue where the enabling/disabling OK button in Create/Edit group was not properly validated with the text boxes present in the window - Group creation not possible anymore with default settings [#4783]


Please remove the changelog entry. This bug was introduced in the dev version and we use the changelog only to inform about changes relative to the latest release (I'll update the PR template to makes this more clear).

The Changelog entry is removed.

…gelistener

Siedlerchr · 2019-03-25T17:36:52Z

            return null;
        });

+        nameField.textProperty().addListener(new ChangeListener<String>() {


You can simplify the changeLIstener with lambdas:

Suggested change

nameField.textProperty().addListener(new ChangeListener<String>() {

nameField.textProperty().addListener((observable, oldValue, newValue) -> upddateComponent());

Hi, Thanks for the suggestion. Working on it now. Will update soon

@Siedlerchr The changes have been committed :)

Siedlerchr · 2019-03-25T17:38:15Z

Please fix the remaining isuses mentioned by @tobiasdiez so we can merge (e.g. remove the action listeners) so we can merge

LinusDietz · 2019-03-26T15:47:27Z

Can you please give the PR a shorter and more decriptive name?

harinda05 · 2019-03-26T17:28:43Z

Can you please give the PR a shorter and more decriptive name?

Done :)

Siedlerchr · 2019-03-27T08:35:10Z

+        searchGroupSearchExpression.textProperty().addListener((observable, oldValue, newValue) -> updateComponents());
+
        EventHandler<ActionEvent> actionHandler = (ActionEvent e) -> updateComponents();
        nameField.setOnAction(actionHandler);


Are these obsolete now?

* upstream/master: (61 commits) fix missing l10n from previous merge fix compile error Fix right clicking on any entry and selecting "Open folder" results in the NullPointer exception (#4797) Bump fontbox from 2.0.14 to 2.0.15 (#4882) Bump pdfbox from 2.0.14 to 2.0.15 (#4881) Bump xmpbox from 2.0.14 to 2.0.15 (#4883) Bump mockito-core from 2.26.0 to 2.27.0 (#4879) Bump java-string-similarity from 1.1.0 to 1.2.1 (#4878) Fix JabRef dying silently without enough inotify instances (#4875) #4795 disable menu item if database not connected (#4828) Remove deprecated awt apple extension (#4860) Fix IllegalArgumentException when ranking entries (#4779) Bump junit-vintage-engine from 5.4.1 to 5.4.2 (#4866) Bump junit-platform-launcher from 1.4.1 to 1.4.2 (#4865) Bump junit-jupiter from 5.4.1 to 5.4.2 (#4867) Add author normalizer for medline import (#4863) Fixed group creation with default settings (#4801) removed default constructor of FXDialogService (#4847) QuotedStringTokenizer now does not unquote (#4830) Bump juh from 5.4.2 to 6.2.2 (#4851) ...

* upstream/master: (184 commits) Try to update to gradle 5.0.2 (#4766) Post change notifications on JavaFX (#4871) fix missing l10n from previous merge fix compile error Fix right clicking on any entry and selecting "Open folder" results in the NullPointer exception (#4797) Bump fontbox from 2.0.14 to 2.0.15 (#4882) Bump pdfbox from 2.0.14 to 2.0.15 (#4881) Bump xmpbox from 2.0.14 to 2.0.15 (#4883) Bump mockito-core from 2.26.0 to 2.27.0 (#4879) Bump java-string-similarity from 1.1.0 to 1.2.1 (#4878) Fix JabRef dying silently without enough inotify instances (#4875) #4795 disable menu item if database not connected (#4828) Remove deprecated awt apple extension (#4860) Fix IllegalArgumentException when ranking entries (#4779) Bump junit-vintage-engine from 5.4.1 to 5.4.2 (#4866) Bump junit-platform-launcher from 1.4.1 to 1.4.2 (#4865) Bump junit-jupiter from 5.4.1 to 5.4.2 (#4867) Add author normalizer for medline import (#4863) Fixed group creation with default settings (#4801) removed default constructor of FXDialogService (#4847) ... # Conflicts: # build.gradle # src/main/java/org/jabref/gui/JabRefFrame.java # src/main/java/org/jabref/gui/collab/ChangeDisplayDialog.java # src/main/java/org/jabref/gui/collab/EntryChangeViewModel.java

harinda05 added 2 commits March 22, 2019 11:49

Fix for the issue - JabRef#4783 Group creation not possible anymore w…

db5b44a

…ith default settings

resolved conflict in GroupDialog.java

212170f

tobiasdiez requested changes Mar 22, 2019

View reviewed changes

harinda05 added 2 commits March 22, 2019 16:25

Changed the position of the listeners and onaction replaced with chan…

87cdd34

…gelistener

Removed changelog entry

f50059a

Siedlerchr reviewed Mar 25, 2019

View reviewed changes

harinda05 added 2 commits March 26, 2019 20:00

Merge branch 'master' of git://github.com/JabRef/jabref

a029969

Fix for issue 4783 - Simplified listeners in to lambdas

01f69c8

harinda05 changed the title ~~Fix for the issue - #4783 Group creation not possible anymore with de…~~ Fixed Group Creation with default settings Mar 26, 2019

harinda05 changed the title ~~Fixed Group Creation with default settings~~ Fixed group creation with default settings Mar 26, 2019

Update GroupDialog.java

0457ccb

tobiasdiez approved these changes Mar 26, 2019

View reviewed changes

tobiasdiez added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Mar 26, 2019

Siedlerchr reviewed Mar 27, 2019

View reviewed changes

Siedlerchr merged commit 8fb663e into JabRef:master Apr 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fixed group creation with default settings#4801

Fixed group creation with default settings#4801
Siedlerchr merged 7 commits into
JabRef:masterfrom
harinda05:master

harinda05 commented Mar 22, 2019

Uh oh!

tobiasdiez left a comment

Uh oh!

tobiasdiez Mar 22, 2019

Uh oh!

harinda05 Mar 22, 2019

Uh oh!

tobiasdiez Mar 22, 2019

Uh oh!

harinda05 Mar 22, 2019

Uh oh!

Siedlerchr Mar 25, 2019

Uh oh!

harinda05 Mar 26, 2019

Uh oh!

harinda05 Mar 26, 2019

Uh oh!

Siedlerchr commented Mar 25, 2019

Uh oh!

LinusDietz commented Mar 26, 2019

Uh oh!

harinda05 commented Mar 26, 2019

Uh oh!

Siedlerchr Mar 27, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	nameField.textProperty().addListener(new ChangeListener<String>() {
	nameField.textProperty().addListener((observable, oldValue, newValue) -> upddateComponent());

Uh oh!

Conversation

harinda05 commented Mar 22, 2019

Uh oh!

tobiasdiez left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Siedlerchr commented Mar 25, 2019

Uh oh!

LinusDietz commented Mar 26, 2019

Uh oh!

harinda05 commented Mar 26, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants