-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Add "Do not ask again" for empty entry confirmation #8767
Conversation
…n check box, add checkbox for dialog service do not ask again in Preference tab, change certain logic in closeTab()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What an effort for a simple check box.
I think, the setting "deleteEmptyEntries" is not necessary to solve the issue.
Localization.lang("Empty entries"), | ||
Localization.lang("Library '%0' has empty entries. Do you want to delete them?", filename), | ||
Localization.lang("Delete"), | ||
Localization.lang("Cancel"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure about this one. Should this be "Keep" or "Keep empty entries"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @koppor , you are right.
It makes much more sense to popup a dialog asking about keeping empty entries instead of deletion.
Please keep me updated if changes needed here.
src/main/java/org/jabref/gui/util/component/TemporalAccessorPicker.java
Outdated
Show resolved
Hide resolved
src/main/java/org/jabref/gui/util/component/TemporalAccessorPicker.java
Outdated
Show resolved
Hide resolved
Localization.lang("Cancel"), | ||
Localization.lang("Do not ask again"), | ||
(optOut) -> prefs.getGeneralPreferences().setConfirmDeleteEmptyEntries(!optOut)); | ||
prefs.getGeneralPreferences().setDeleteEmptyEntries(response); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This changes the user's preferences. The PR did not show a screenshot of that setting.
If I (as user) am asked whether I want to delete empty entries, what happens the next time I press on save? Is my last setting preserved?
I this is used only for the current call of write, this should be done differently (maybe as it was before)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @koppor , I am not too sure about your question.
- If user does not tick on "Do not ask again" checkbox, system will keep popping the confirmation dialog about empty entries deletion.
- If user does tick on "Do not ask again" checkbox,
prefs.getGeneralPreferences().setConfirmDeleteEmptyEntries
used to store the value and no longer show the confirmation dialog in future - If user clicks on "Delete" button and tick on "Do not ask again" checkbox,
prefs.getGeneralPreferences().setDeleteEmptyEntries(response)
always delete empty entries in future. - If user does not click on "Delete" button but tick on "Do not ask again" checkbox,
prefs.getGeneralPreferences().setDeleteEmptyEntries(response)
always does not delete empty entries in future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the explanation. I read that the preference is always saved, independent of the checkbox "Do not ask again". This is IMHO the first occasion where during a usage of JabRef the preferences are changed. (Side note: It is OK for me, because I used that concept a decade ago. However, we removed that approx. 5 years ago not to confuse users)
With "did not show a screenshot" I meant that "Always delete entries" is not showing up at https://user-images.githubusercontent.com/49628911/167340402-3bde988e-d67e-4734-bf8f-3b4992ed219d.png - or do I miss something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your review @koppor.
In my humble opinion, "Always delete entries" preference relies on the empty entries deletion confirmation dialog. Therefore, I have only added "Show confirmation dialog when deleting empty entries" for user to access the preference of confirmation dialog.
Could you please advice if "Always delete entries" is needed at https://user-images.githubusercontent.com/49628911/167340402-3bde988e-d67e-4734-bf8f-3b4992ed219d.png?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@koppor Not sure, we have a couple of confirmation dialogs where the choice is stored in the preferences
I put the devcall label to get this PR decided (and to discuss #8296 (comment)) |
We had a discussion in today's developer's call and decided to go a different way. See #8645 (comment). Thus, this PR is obsolete. We hope you learned something about Java and Open Source nevertheless. |
Previous discussion about add "Do not ask again" for empty entry confirmation in JabRef #8296, related to #8096
Proposed solution:
Fixes #8296
Do not ask again
in dialog box when user closes a tab or quit.Do not ask again
checkbox is ticked and:Show confirmation dialog when deleting empty entries
(Options -> Preferences -> General)PR Checklist:
CHANGELOG.md
described in a way that is understandable for the average user (if applicable)