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

Fix libre office connection and other progress dialogs #6478

Merged
merged 3 commits into from
May 14, 2020
Merged

Conversation

Siedlerchr
Copy link
Member

@Siedlerchr Siedlerchr commented May 14, 2020

Fixes #6477

Error was introduced in #6443 with the ProgressDialog showAndWait
I had to revert that. @btut
The showAndWait broke all dialogs which used the dialog. (E.g). also copy linked files)

  • Change in CHANGELOG.md described (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.

@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label May 14, 2020
@btut
Copy link
Contributor

btut commented May 14, 2020

Uh I forgot to roll that back. The ProgressDialog is not actually used by the background-task functionality because I could not get it to work.
I think it would be better to change the name of the method to showProgressDialog. The old name, showProgressDialogAndWait, suggests that the dialog is waited for.

@Siedlerchr
Copy link
Member Author

Indeed, good idea. I renamed it now

@tobiasdiez tobiasdiez merged commit 46777c2 into master May 14, 2020
@tobiasdiez tobiasdiez deleted the fixLO branch May 14, 2020 09:07
@calixtus
Copy link
Member

Do I get that right, that most of the progress dialogs can probably be converted to be displayed in the new progress popup?

@btut
Copy link
Contributor

btut commented May 14, 2020

I don't know which other features use the progress dialogue.
I had three problems with the progress dialogue:

  • It displays the progress of an actual task. I wanted to show a list of tasks
  • It is not waited for
  • There is no way to close it when the progress reached 100%

In the dialogue that I implemented as a solution for this, I used the list of tasks stored in StateManager. It would be easy to pass a list of tasks instead. Then we could add an option to the dialogue to only show and it could replace the other dialogues.

But as the original dialogue is not waited for, I guess it is only used as information to the user. How about registering the relevant task in the background-task list in StateManager? Then you could just pop up the new dialogue. You could even pop up the pop-over of the task-progress-indicator in the toolbar. This would be very similar to a show-only dialog (no wait). The user can hide it and check the status later by clicking on the progress-indicator again.

IMO this would be a good way to go. I think the new dialogue and pop-over are more informative as they show the actual progress of tasks if available. AFAICT from the code the old dialogue just showed an indeterminate progress bar.

@btut
Copy link
Contributor

btut commented May 14, 2020

... I just checked and it is just the WaitForSaveFinished dialog that creates a task for this purpose. The other times the dialogue is shown it is shown with actual worker tasks, so progress should be shown.

@tobiasdiez
Copy link
Member

I agree these progress dialogs can be replaced by using the new progress popup. None of them actually has a reason to block the UI and can safely be run in the background in my opinion.

@Siedlerchr
Copy link
Member Author

The only critical application is indeed the connection to LibreOffie, but that could also be done in a background task because it's usually finished very fast. However, we have to be careful of tasks throwing exceptions.
The copy linked files to folder displays a progress dialog and afterwards the result dialog.
Indeed it doesn't make sense to block the UI with a progress dialog here

Siedlerchr added a commit that referenced this pull request May 16, 2020
* upstream/master: (50 commits)
  Keep group pane size when resizing window (#6180) (#6423)
  Changelog: Fix missing citation for biblatex-mla
  Update AUTHORS
  Check duplicate DOI (#6333)
  Fix missing citation for biblatex-mla
  Change EasyBind dependency (#6480)
  Add testing of latest dev version as mandatory
  Squashed 'src/main/resources/csl-styles/' changes from 5dad23d..586e0b8
  Fix libre office connection and other progress dialogs (#6478)
  Fix clear year and month field when converting to biblatex (#6434)
  Add truncate as a BibTex key modifier (#6427)
  Add new authors (not all - they need more work)
  Remove empty line
  Add simple Unit Tests for #6207 (#6240)
  Enforce LeftCurly rule (#6452)
  Implement task progress indicator (and dialog) in the toolbar (#6443)
  Consider empty brackets
  Changelog update
  Added a test
  Fixed brackets in regular expressions
  ...
koppor pushed a commit that referenced this pull request Apr 1, 2023
41531558a8 Fix unsigned newspaper articles throughout Chicago 17 (#6486)
7678212826 Create trames.csl (#6479)
0cae26ac85 Update hochschule-fur-soziale-arbeit-fhnw.csl (#6480)
85c4b693a2 Update to UP Harvard Theology & Religion (#6485)
c273aa7e43 Update ieee.csl (#6481)
fe67b80e47 Update open-window.csl (#6367)
f2229705ef Create iainutuban-tarbiyah.csl (#6361)
1867a56a26 Create business-and-human-rights-journal (#6359)
1371dbdf26 Update iso690-author-date-es.csl (#6477)
6953a43efd Update ieee.csl (#6478)
f56d5ef1cc Create czech-journal-of-international-relations.csl (#6453)
678b53f99c Update harvard-stellenbosch-university.csl (#6464)
3074938038 Update ucl-university-college-apa.csl (#6475)
27dab9ea0f Update iso690-author-date-es.csl (#6476)
a8aea63d00 Create elsevier-american-chemical-society.csl (#6342)
f8f290fa63 Update iso690-author-date-es.csl (#6472)
7fdc621eee Update journal-of-neolithic-archaeology (#6466)
7025568e70 Update offa.csl (#6465)
2d69299b19 Create uni-fribourg-theologie.csl (#6473)
8db531a73e Create travail-et-emploi.csl (#6351)
c8b54fc531 Make monash-university-harvard dependent style (#6470)
b95f59ff5c Update journal-of-the-marine-biological-association-of-the-united-kingdom.csl (#6456)
a12b513119 Update universite-du-quebec-a-montreal.csl (#6463)
048e6641e4 Update zeitschrift-fur-geschichtsdidaktik.csl (#6454)
f0d3d7ef15 Update journal-fur-kulturpflanzen-journal-of-cultivated-plants.csl (#6447)
3b814fe048 Update the-accounting-review.csl (#6459)
f24befd580 Update survey-of-ophthalmology.csl from ama.csl to its own independent style (#6460)
c868ab54f6 Create vancouver-alphabetical.csl (#6461)
782e39cfe1 Update american-institute-of-physics.csl (#6457)
a56cf03e3c Fix Chicago Cases & Newspaper sorting (#6458)

git-subtree-dir: buildres/csl/csl-styles
git-subtree-split: 41531558a873b2533f2d17d8d6484c2408174fce
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.

Libreoffice connection broken in 5.1
4 participants