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.

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

Adds progress count to Possible Duplicates dialog #7602

Merged

Conversation

tp-1000
Copy link
Contributor

@tp-1000 tp-1000 commented Mar 30, 2021

resolves #7366

When processing duplicate entries,
Possible Duplicates dialog gave no indication as to progress.

To address this lack of feedback,
a progress counter was added to the title bar.

The title property now contains two updatable properties:

  1. A total count of all duplicates
  2. a count of how many duplicates have already been addressed

They are updated with listeners and bindings to
provide real time feedback.


Note: because of the use of different threads, the total-count could not be directly bound so an extra variable and listener are required. Also line 88 was removed and duplicateCount.getAndIncrement() was changed from getAndIncrement() to incrementAndGet() so returned value could be used.


Screen Shot 2021-03-30 at 4 16 45 PM

  • 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 created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

resolves JabRef#7366

When processing duplicate entries,
Possible Duplicates dialog gave no indication as to progress.

To address this lack of feedback,
a progress counter was added to the title bar.

The title property now contains two updatable properties:
1) A total count of all duplicates
2) a count of how many duplicates have already been addressed

They are updated with listeners and bindings to
provide real time feedback.
@Siedlerchr
Copy link
Member

The Background Tasks have already a ProgessValue and progressText property you can bind to, have a look at the umportFilesInBackground here

public BackgroundTask<List<ImportFilesResultItemViewModel>> importFilesInBackground(List<Path> files) {
return new BackgroundTask<>() {
private int counter;
private List<BibEntry> entriesToAdd;
private final List<ImportFilesResultItemViewModel> results = new ArrayList<>();
@Override
protected List<ImportFilesResultItemViewModel> call() {
counter = 1;
CompoundEdit ce = new CompoundEdit();
for (Path file: files) {
entriesToAdd = Collections.emptyList();
if (isCanceled()) {
break;
}
DefaultTaskExecutor.runInJavaFXThread(() -> {
updateMessage(Localization.lang("Processing file %0", file.getFileName()));
updateProgress(counter, files.size() - 1);
});

And the binding in UnlinkedFilesDialog

@tobiasdiez
Copy link
Member

@Siedlerchr I think this is a different kind of progress. Here the user is clicking and thus changing the "progress" while for your solution something is happening in the background.

@tp-1000
Copy link
Contributor Author

tp-1000 commented Mar 31, 2021

@Siedlerchr The progress count is indeed a user paced progress, which changes as the user clicks options for resolving duplicates and in a few cases increments automatically without presenting a user a dialog window but still only as a response to a previous dialog click. This value I didn't think would be appropriate to use the background task.

The other value, the total, is only a count of total pairs found. While it may be possible to take advantage of the background progress monitoring, it's really not a true progress count that is needed for this value. I would have liked to simply request the value from the AtomicInteger duplicateCount (like the notify at the end) but I wasn't able to use this directly because the dialog pops up before many of the pairs have been made often giving an artificially low count of 1/1.

This ends up being a progress count of a user working through the end result of the of the find-possible-pairs task.

@Siedlerchr
Copy link
Member

Ah thanks for the explanation

@tp-1000
Copy link
Contributor Author

tp-1000 commented Apr 3, 2021

@Siedlerchr would you like to see revisions?

Copy link
Member

@Siedlerchr Siedlerchr left a comment

Choose a reason for hiding this comment

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

Thanks again for your work, lgtm so far

@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Apr 3, 2021
@koppor koppor merged commit c06966d into JabRef:main Apr 12, 2021
@Siedlerchr
Copy link
Member

@tp-1000 Thanks for your contribution! And sorry for the delay!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add duplicate count/progress to Possible Duplicates dialog
4 participants