Enable editing typo with double clicking on field name in custom entry type #9977

DinjerChang · 2023-06-06T00:45:32Z

References #9840

This PR is related to the second todo in #9840
We enable the editing feature for user when they have typo when customizing the new field.

Mandatory checks

Change in CHANGELOG.md described in a way that is understandable for the average user (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 developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

mrege jabref main to dinjer jabref main

…he fieldname to backend

….java

koppor · 2023-06-06T04:28:59Z

src/main/java/org/jabref/gui/preferences/customentrytypes/CustomEntryTypesTab.java

+                    } else {
+                        dialogService.showWarningDialogAndWait(
+                                Localization.lang("Duplicate fields"),
+                                Localization.lang("Warning: You added field \"%0\" twice. Only one will be kept.", newFieldValue));
+                        event.getTableView().edit(-1, null);
+                        event.getTableView().refresh();
+                    }


Modern software seldom makes use of popups. I would use the notifiy functionality. The user has no choice. Moreover, the current code keeps the old casing. Thus, instead of newFieldValue, the existing one should be output. As a consequence, the code of fieldExists needs to be modified accordingly.

The reason I use newFieldValue instead of the existing one is to see if the user input value (newFieldValue) exist in the current fields. Let's say we already have two new Field, Name and Test. When a user try to edit Name to Test, he would double click the Name Field and type "Test", so "Test" is the newFieldValue. So I think we have to see what user input is and compare it with the existing Field. In this case, Test already exist, so we will notify the user.

If we choose the existing one by using the method field.getDisplayName(), the value would be Name since we haven't modify the field yet. In this case, we always get the fieldExists value to be True

Also, I'm replacing equals with equlsIgnoreCase to handle the casing comparison problem.
Let's say a user edit Name to name, it should be and will be seen as duplicate field.

Let me know if I understand statement correctly and if I'm on the right track :)

Sorry that I was not clear in my terms. Thank you for explaining a scenario. I hope, this short explanation based on your scenario helps:

I was trying to say: User renames to TEST, but Test exists. I assume the user knows, he input TEST, but forgot about the existing field Test. Therefore, Test should be displayed and not TEST.

…ing user input value with existing value of field

koppor

One more thing:

Double click on a field
Press enter

Expected behavior: Field is as kept as is - and no notificaiton is output

Actual behavior: Field is kapt AND an notification is output:

I think, in if (fieldExists) inside an additional check needs to be made. Maybe also before - if the old name is the new name

src/main/java/org/jabref/gui/preferences/customentrytypes/CustomEntryTypesTab.java

…fter double click then click enter, change negataion to positvie of fieldExists

koppor · 2023-06-08T04:31:34Z

src/main/java/org/jabref/gui/preferences/customentrytypes/CustomEntryTypesTab.java

                    boolean fieldExists = entryFields.stream().anyMatch(fieldViewModel ->
-                            fieldViewModel.nameProperty().getValue().equalsIgnoreCase(newFieldValue));
+                            fieldViewModel.nameProperty().getValue().equalsIgnoreCase(newFieldValue) && !newFieldValue.equals(field.getDisplayName()));


The new condition does not make use of ´fieldViewModel `

I think, following code matches your intention:

boolean fieldExists = !newFieldValue.equals(field.getDisplayName()) && entryFields.stream().anyMatch(fieldViewModel -> fieldViewModel.nameProperty().getValue().equalsIgnoreCase(newFieldValue));

You're right. I shouldn't put it in the anyMatch() method when streaming

…layName())

…abref into fix-for-issue-9840

koppor

Thank you for keeping working on this!

I think, UI tests for this are hard. Thus, this PR is OK from my side!

koppor · 2023-06-08T14:04:03Z

@DinjerChang In future, please chechk if an issue is assigned to a person and if the person is on your team. It seams that the issue was assigned to @eric052199, which probably is not on your team? -- Do you intend to work on the first item of the TODO, too?

calixtus · 2023-06-08T15:08:06Z

Modified changelog, since this was not a fix for a bug, but a small and most welcome enhancement. Thanks!

DinjerChang · 2023-06-08T15:25:34Z

@koppor @calixtus Sorry I forgot to mention that Eric and I are on the same team.
I'll take some time researching on the first todo to see if I could fix it!
Another subject, when working on the second todo, I took some references of the file org/jabref/gui/preferences/citationkeypattern/CitationKeyPatternTab.java line 66 ~ 68 and
org/jabref/gui/preferences/customentrytypes/CustomEntryTypesTabViewModel.java #addNewField().
I think maybe those parts need some refactoring like the dialog.notify() and the if-else negation statement of fieldExists that we modified in this PR, just for your information.
Overall, thanks for the heads up!

Siedlerchr · 2023-06-08T19:00:21Z

@DinjerChang That sounds good. I would suggest you create a new PR then and we merge this one

koppor · 2023-06-08T19:47:01Z

When the user entered the same field twice, the old field value is kept. Thus, the notification message is wrong. We fixed that.

koppor · 2023-06-09T00:48:34Z

While discussing it with a group of maintainers of JabRef, we worked on the code again: PR is #9993. We hope to get this merged the next days, so that you have a good foundation to work on the core issue.

For the different field properties, a good start are tests. A skeletton is provided at https://github.com/JabRef/jabref/pull/9993/files#diff-420e1dfef38c9004e74cc9b2074499e837558303f00e177c9312ccd38def62e1. It could be used to work on the serialization format proposed at step 10 at #9840 (comment).

* upstream/main: (68 commits) Fix issue 9863 - Change Tab selection order (#9907) New Crowdin updates (#9994) Enable editing typo with double clicking on field name in custom entry type (#9977) Remove "Field name:" from localization - we already have "Field name" (#9991) Changed database to catalog in org.jabref.gui.slr and org.jabref.logic.crawler (#9989) Use environment variables for hostname detection (#9910) Add cleanup activity for URL field (#9970) Fix freezing when fetching IBSN and no results are found (#9987) Make Group(Node)TreeViewModel more OO (#9978) Fix container for group item count still visible if display count is off (#9980) Fix paste of BibTeX data (#9985) Bring back SimplifyBoolean* and UnnecessaryParantheses (and refine guide) (#9981) Add new menu entry to remove braces from selection, aka unprotect it. (#9968) User specific comment (#9727) Bump org.openrewrite.recipe:rewrite-recipe-bom from 1.19.3 to 2.0.0 (#9975) Fix typos Remove non-needed link Fix typos Enable RemoveJavaDocAuthorTag (#9973) Bump com.fasterxml.jackson.datatype:jackson-datatype-jsr310 (#9974) ...

DinjerChang and others added 5 commits June 4, 2023 17:55

Merge pull request #1 from JabRef/main

b7b6b49

mrege jabref main to dinjer jabref main

frontend custom entry type edit feature

ab673dc

new function setName() in UnkownField.java added so we could update t…

fc6c245

…he fieldname to backend

CHANGELOG.MD added

ed7373c

checkstyle fixed, better varible name modified in CustomEntryTypesTab…

8a2daa0

….java

koppor requested changes Jun 6, 2023

View reviewed changes

dialog with notify functionality and use equalsIgnoreCase when compar…

d653fc5

…ing user input value with existing value of field

ThiloteE added ui bib(la)tex labels Jun 6, 2023

DinjerChang added 2 commits June 6, 2023 17:14

add Localization.lang("key") to JabRed_en.properties

2be4ccb

Capitalizing first letter when displaying on coulumn after edit

b6cd9b7

koppor requested changes Jun 7, 2023

View reviewed changes

src/main/java/org/jabref/gui/preferences/customentrytypes/CustomEntryTypesTab.java Show resolved Hide resolved

src/main/java/org/jabref/gui/preferences/customentrytypes/CustomEntryTypesTab.java Outdated Show resolved Hide resolved

DinjerChang and others added 2 commits June 7, 2023 10:44

fix when user input the original field name or doesnt edit anything a…

e7d288c

…fter double click then click enter, change negataion to positvie of fieldExists

Merge branch 'main' into fix-for-issue-9840

6d4e751

koppor requested changes Jun 8, 2023

View reviewed changes

DinjerChang added 2 commits June 7, 2023 21:43

fix wrong placement of predicate: !newFieldValue.equals(field.getDisp…

574cd4e

…layName())

Merge branch 'fix-for-issue-9840' of https://github.com/DinjerChang/j…

45681d9

…abref into fix-for-issue-9840

koppor previously approved these changes Jun 8, 2023

View reviewed changes

koppor added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jun 8, 2023

Merge branch 'main' into fix-for-issue-9840

66bc727

koppor dismissed their stale review via 66bc727 June 8, 2023 14:02

koppor mentioned this pull request Jun 8, 2023

Enable casing for adding fields for entry types #9945

Closed

Update CHANGELOG.md

ee41ca9

Siedlerchr previously approved these changes Jun 8, 2023

View reviewed changes

koppor dismissed Siedlerchr’s stale review via d19f84f June 8, 2023 19:46

Fix localization

d19f84f

Siedlerchr approved these changes Jun 8, 2023

View reviewed changes

calixtus merged commit 5ca1ad8 into JabRef:main Jun 8, 2023

koppor mentioned this pull request Jun 9, 2023

Preserve casing of field names for customize entry types #9993

Merged

6 tasks

DinjerChang deleted the fix-for-issue-9840 branch June 18, 2023 20:51

DinjerChang restored the fix-for-issue-9840 branch June 18, 2023 20:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable editing typo with double clicking on field name in custom entry type #9977

Enable editing typo with double clicking on field name in custom entry type #9977

DinjerChang commented Jun 6, 2023 •

edited by koppor

Loading

koppor Jun 6, 2023

DinjerChang Jun 6, 2023 •

edited

Loading

koppor Jun 6, 2023

koppor left a comment

koppor Jun 8, 2023

DinjerChang Jun 8, 2023

koppor left a comment

koppor commented Jun 8, 2023

calixtus commented Jun 8, 2023

DinjerChang commented Jun 8, 2023 •

edited

Loading

Siedlerchr commented Jun 8, 2023

koppor commented Jun 8, 2023

koppor commented Jun 9, 2023

Enable editing typo with double clicking on field name in custom entry type #9977

Enable editing typo with double clicking on field name in custom entry type #9977

Conversation

DinjerChang commented Jun 6, 2023 • edited by koppor Loading

Mandatory checks

koppor Jun 6, 2023

Choose a reason for hiding this comment

DinjerChang Jun 6, 2023 • edited Loading

Choose a reason for hiding this comment

koppor Jun 6, 2023

Choose a reason for hiding this comment

koppor left a comment

Choose a reason for hiding this comment

koppor Jun 8, 2023

Choose a reason for hiding this comment

DinjerChang Jun 8, 2023

Choose a reason for hiding this comment

koppor left a comment

Choose a reason for hiding this comment

koppor commented Jun 8, 2023

calixtus commented Jun 8, 2023

DinjerChang commented Jun 8, 2023 • edited Loading

Siedlerchr commented Jun 8, 2023

koppor commented Jun 8, 2023

koppor commented Jun 9, 2023

DinjerChang commented Jun 6, 2023 •

edited by koppor

Loading

DinjerChang Jun 6, 2023 •

edited

Loading

DinjerChang commented Jun 8, 2023 •

edited

Loading