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

Fixed #6472 that multiple background task popups stacked over each other #6475

Merged
merged 9 commits into from
May 31, 2020

Conversation

chenyuheng
Copy link
Contributor

Fixed #6472

I added a globle PopOver variable to hide existed PopOver(if any).

If a pop over is showing and I clicked on the background tasks button, the showing pop over will hide first, then the new pop over will show. The pop overs won't stack over.
image

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

add concise tips when SaveException happend
fixed the problem that multiple background task popups stacked over each other.

Added a globle PopOver variable to hide existed PopOver(if any).
@chenyuheng chenyuheng changed the title Fixed https://github.com/JabRef/jabref/issues/6472 Fixed #6472 that multiple background task popups stacked over each other May 13, 2020
@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label May 13, 2020
Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix! I've one idea, can you please try if it also works that way.

progressViewPopOver.setTitle(Localization.lang("Background Tasks"));
progressViewPopOver.setArrowLocation(PopOver.ArrowLocation.RIGHT_TOP);

this.existedProgressViewPopOver.hide();
Copy link
Member

Choose a reason for hiding this comment

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

Another option would be to reuse the existing pop over and only change the content. That is:

if (existedProgressViewPopOver == null) {
   existedProgressViewPopOver = new ...
} else {
   existedProgressViewPopOver.setContentNode(taskProgressView).
}

This should be a slightly better user experience as there is no flicker of hiding/showing a new pop over.

Then you can also rename the global variable to progressViewPopOver

Copy link
Member

Choose a reason for hiding this comment

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

In this context, it would be nice if the progress button serves as a toggle: if the pop over is currently shown, and the button is clicked again then the pop over should be hidden.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's great! I have made the change, it works.

@tobiasdiez tobiasdiez added the status: changes required Pull requests that are not yet complete label May 14, 2020
@Siedlerchr
Copy link
Member

It would be really nice if you could finish this PR and try the changes so that we can merge this PR soon.

@chenyuheng
Copy link
Contributor Author

It would be really nice if you could finish this PR and try the changes so that we can merge this PR soon.

Yes, sorry for the late change. I have made that, please check it again!

Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the follow-up.

@tobiasdiez tobiasdiez removed the status: changes required Pull requests that are not yet complete label May 31, 2020
@Siedlerchr Siedlerchr merged commit 455ed8b into JabRef:master May 31, 2020
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.

Multiple background task popups stacked over each other
3 participants