Skip to content

Adds progress count to Possible Duplicates dialog#7602

Merged
koppor merged 1 commit into
JabRef:mainfrom
tp-1000:enhancement-duplicate-count-progress-update-7366
Apr 12, 2021
Merged

Adds progress count to Possible Duplicates dialog#7602
koppor merged 1 commit into
JabRef:mainfrom
tp-1000:enhancement-duplicate-count-progress-update-7366

Conversation

@tp-1000

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

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
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

tp-1000 commented Mar 31, 2021

Copy link
Copy Markdown
Contributor Author

@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
Copy Markdown
Member

Ah thanks for the explanation

@tp-1000

tp-1000 commented Apr 3, 2021

Copy link
Copy Markdown
Contributor Author

@Siedlerchr would you like to see revisions?

@Siedlerchr Siedlerchr left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
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