Add "Do not ask again" for empty entry confirmation #8297

ruoyu-qian · 2021-12-05T08:26:02Z

Fixes #8296, a follow-up from #8096 (comment)

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 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.

calixtus · 2021-12-05T21:34:16Z

src/main/java/org/jabref/preferences/GeneralPreferences.java

+    /**
+     * Get the preference of whether to give a confirmation dialog when empty entries are detected in the library
+     *
+     * @return true if the preference is set as yes
+     */
+    public boolean shouldConfirmEmptyEntries() {
+        return confirmEmptyEntries.get();
+    }
+
+    /**
+     * Get the BooleanProperty of the preference of whether to give a confirmation dialog when empty entries are detected in the library
+     *
+     * @return BooleanProperty
+     */
+    public BooleanProperty confirmEmptyEntriesProperty() {
+        return confirmEmptyEntries;
+    }
+
+    /**
+     * Set the preference of whether to give a confirmation dialog when empty entries are detected in the library
+     *
+     * @param confirmEmptyEntries        boolean for the preference
+     */
+    public void setConfirmEmptyEntries(boolean confirmEmptyEntries) {
+        this.confirmEmptyEntries.set(confirmEmptyEntries);
+    }
+
+    /**
+     * Get the preference of whether to delete the empty entries detected in the library if confirmation dialog is opted out
+     *
+     * @return true if the preference is set as yes (delete)
+     */
+    public boolean shouldDeleteEmptyEntries() {
+        return deleteEmptyEntries.get();
+    }
+
+    /**
+     * Get the BooleanProperty of the preference of whether to delete the empty entries detected in the library if confirmation dialog is opted out
+     *
+     * @return BooleanProperty
+     */
+    public BooleanProperty deleteEmptyEntriesProperty() {
+        return deleteEmptyEntries;
+    }
+
+    /**
+     * Set the preference of whether to delete the empty entries detected in the library if confirmation dialog is opted out
+     *
+     * @param deleteEmptyEntries        boolean for the preference
+     */
+    public void setDeleteEmptyEntries(boolean deleteEmptyEntries) {
+        this.deleteEmptyEntries.set(deleteEmptyEntries);
+    }


This javadoc is superfluous. Please comment on the the variable or the property, but not on every getter and setter

Sorry about this. I removed these comments. Let me know if you would like have any other changes.

calixtus · 2021-12-05T21:44:35Z

src/main/java/org/jabref/gui/JabRefFrame.java

@@ -1181,11 +1191,22 @@ private Boolean confirmEmptyEntry(LibraryTab libraryTab, BibDatabaseContext cont
        ButtonType keepEmptyEntries = new ButtonType(Localization.lang("Keep empty entries"), ButtonBar.ButtonData.NO);
        ButtonType cancel = new ButtonType(Localization.lang("Return to JabRef"), ButtonBar.ButtonData.CANCEL_CLOSE);

-        Optional<ButtonType> response = dialogService.showCustomButtonDialogAndWait(Alert.AlertType.CONFIRMATION,
+        Optional<ButtonType> response = dialogService.showCustomButtonDialogWithOptOutAndWait(Alert.AlertType.CONFIRMATION,


Why don't you use the showConfirmationDialogWithOptOutAndWait? The AlertType.CONFIRMATION is used there too.
This looks to me like unnecessary code duplication, or didn't I see anything?

The reason I added showCustomButtonDialogWithOptOutAndWait is because I would like to include the 'Return to JabRef' option as it has now. I believe showConfirmationDialogWithOptOutAndWait only allows for Yes/No choice, but not the third option.

ThiloteE · 2021-12-19T01:34:03Z

Would this make it into stable 5.4?

Siedlerchr

From my point of view it's good to go. Waiting for final approval from @calixtus

calixtus

Hi, I took a look into your PR again, there is still an issue with some code duplication.
Also I tested your PR manually and it did not work as expected.

Create file with just @Article{, }
Import as new library
Close again
Dialog shows up
Select return to JabRef
Close tab
Tab wont close
Select other tab
Tab cannot be selected again

Please look into these issues.

calixtus · 2021-12-19T19:24:35Z

src/main/java/org/jabref/gui/JabRefDialogService.java

+    @Override
+    public Optional<ButtonType> showCustomButtonDialogWithOptOutAndWait(Alert.AlertType type, String title, String content,
+                                                                        String optOutMessage, Consumer<Boolean> optOutAction,
+                                                                        ButtonType... buttonTypes) {
+        FXDialog alert = createDialogWithOptOut(AlertType.CONFIRMATION, title, content, optOutMessage, optOutAction);
+        alert.getButtonTypes().setAll(buttonTypes);
+        return alert.showAndWait();
+    }
+


There still seems to be some code duplication with showConfirmationDialogWithOptOutAndWait, which is basically now a subset of this method here. Should call this instead.
Also the method argument type in the method signature, but is not used in the method.

calixtus

.

Siedlerchr

The tab gets blocked. Please investigate

calixtus · 2022-03-29T21:27:43Z

Closing this pull request due to inactivity 💤
Please reopen this PR if you are going to fix the bugs if you like, we would love to see this feature finally implemented.

ThiloteE · 2022-05-06T15:39:28Z

Another issue with the current implementation: #8645
Maybe 8645 could also be addressed, when finalizing this pull-request. Could of course also be done in a separate pull-request.

ruoyu-qian added 7 commits December 4, 2021 19:10

Added EmptyEntryTab

44b3b4b

Update JabRefPreferences.java

91335d3

Update GeneralPreferences.java

dbfd569

Removed previous edits

f6caf3a

Update GeneralPreferences.java

cd195cd

Update JabRefFrame.java

bebbc90

Update JabRefFrame.java

4eed604

Siedlerchr requested a review from calixtus December 5, 2021 12:18

Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Dec 5, 2021

ruoyu-qian added 9 commits December 5, 2021 10:52

add showCustomButtonDialogWithOptOutAndWait()

9388fc8

Update JabRefFrame.java

6c15642

Update JabRefFrame.java

8cd129e

Update GeneralPreferences.java

e4779fd

Update JabRefPreferences.java

9e1ac51

Update JabRefFrame.java

c1303fb

Update JabRefFrame.java

66cb560

edited Javadoc comment

a6c7636

Update JabRefFrame.java

081fe2a

calixtus reviewed Dec 5, 2021

View reviewed changes

ruoyu-qian added 4 commits December 5, 2021 20:51

Update GeneralPreferences.java

074f3df

Update JabRefFrame.java

6023c7b

Update DialogService.java

0fb91b0

Update DialogService.java

50750a9

Siedlerchr approved these changes Dec 19, 2021

View reviewed changes

Siedlerchr added status: changes required Pull requests that are not yet complete and removed status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers labels Dec 19, 2021

calixtus reviewed Dec 19, 2021

View reviewed changes

calixtus requested changes Dec 19, 2021

View reviewed changes

Siedlerchr requested changes Dec 19, 2021

View reviewed changes

Siedlerchr assigned calixtus Jan 3, 2022

calixtus closed this Mar 29, 2022

LIM0000 mentioned this pull request May 6, 2022

Add "Do not ask again" for empty entry confirmation in JabRef #8296

Closed

LIM0000 added a commit to LIM0000/jabref that referenced this pull request May 9, 2022

Revert fix for JabRef#8297, only focus on JabRef#8296

3a30392

LIM0000 mentioned this pull request May 9, 2022

Library tab unable to close after unsave action #8768

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add "Do not ask again" for empty entry confirmation #8297

Add "Do not ask again" for empty entry confirmation #8297

ruoyu-qian commented Dec 5, 2021 •

edited

Loading

calixtus Dec 5, 2021

ruoyu-qian Dec 6, 2021

calixtus Dec 5, 2021

ruoyu-qian Dec 6, 2021

ThiloteE commented Dec 19, 2021

Siedlerchr left a comment

calixtus left a comment •

edited by Siedlerchr

Loading

calixtus Dec 19, 2021

calixtus left a comment

Siedlerchr left a comment

calixtus commented Mar 29, 2022 •

edited

Loading

ThiloteE commented May 6, 2022

Add "Do not ask again" for empty entry confirmation #8297

Add "Do not ask again" for empty entry confirmation #8297

Conversation

ruoyu-qian commented Dec 5, 2021 • edited Loading

calixtus Dec 5, 2021

Choose a reason for hiding this comment

ruoyu-qian Dec 6, 2021

Choose a reason for hiding this comment

calixtus Dec 5, 2021

Choose a reason for hiding this comment

ruoyu-qian Dec 6, 2021

Choose a reason for hiding this comment

ThiloteE commented Dec 19, 2021

Siedlerchr left a comment

Choose a reason for hiding this comment

calixtus left a comment • edited by Siedlerchr Loading

Choose a reason for hiding this comment

calixtus Dec 19, 2021

Choose a reason for hiding this comment

calixtus left a comment

Choose a reason for hiding this comment

Siedlerchr left a comment

Choose a reason for hiding this comment

calixtus commented Mar 29, 2022 • edited Loading

ThiloteE commented May 6, 2022

ruoyu-qian commented Dec 5, 2021 •

edited

Loading

calixtus left a comment •

edited by Siedlerchr

Loading

calixtus commented Mar 29, 2022 •

edited

Loading