Wrapper classes for UI Lists etc that ensure updates on JavaFX thread #5289

r0light · 2019-09-08T21:02:58Z

Furthermore, the threading issue part of #4958 is fixed by using UiThreadStringProperty

New classes are added to gui.util.uithreadaware which enable a wrapper class around ObservableList which ensures that changes are propagated on the JavaFX Application thread.

I started with UiThreadListDecorator, but others like UiThreadMapDecorator could be implemented similarly.

Change in CHANGELOG.md described
Tests created for changes
Manually tested changed features in running JabRef
Screenshots added in PR description (for bigger UI changes)
Ensured that the git commit message is a good one
Check documentation status (Issue created for outdated help page at help.jabref.org?)

tobiasdiez

Nice! That's exactly what I had in mind.

It would be cool if you could add a similar wrapper in order to also fix the similar issue #4958. (If you don't have the time for it at the moment, then it is also fine to do this later.)

tobiasdiez · 2019-09-08T21:53:24Z

src/main/java/org/jabref/gui/util/uithreadaware/UiThreadHelper.java

+
+import javafx.application.Platform;
+
+public class UiThreadHelper {


make this package private

done in 96b9aa5

tobiasdiez · 2019-09-08T21:53:56Z

src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditorViewModel.java

@@ -43,7 +44,7 @@

 public class LinkedFilesEditorViewModel extends AbstractEditorViewModel {

-    private final ListProperty<LinkedFileViewModel> files = new SimpleListProperty<>(FXCollections.observableArrayList(LinkedFileViewModel::getObservables));
+    private final ListProperty<LinkedFileViewModel> files = new SimpleListProperty<>(new UiThreadListDecorator<>(FXCollections.observableArrayList(LinkedFileViewModel::getObservables)));


Please use the wrapper not in the view model (which may e.g be used in tests) but in the view class.

ok, done in 96b9aa5

…y in View not in Model

r0light · 2019-09-09T09:03:47Z

Ok, it took me while to reproduce #4958 reliably but now i added a UiThreadStringProperty in PersonsEditor which wraps the original property and the issue should be fixed now.

@tobiasdiez should we use as few as possible new UI thread aware classes only where necessary or should i add it consistently to classes in gui.fieldeditors ?

tobiasdiez · 2019-09-09T10:36:35Z

src/main/java/org/jabref/gui/fieldeditors/PersonsEditor.java

@@ -28,7 +30,8 @@ public PersonsEditor(final Field field,
                ? new EditorTextField()
                : new EditorTextArea();

-        textInput.textProperty().bindBidirectional(viewModel.textProperty());
+        decoratedStringProperty = new UiThreadStringProperty(viewModel.textProperty());


Is it necessary to keep a track of the reference? If not I would prefer if decoratedStringProperty is converted to a local variable (or be inlined completely).

In fact, i tried it with an inlined approach first, but the list of files did not update itself then.
I did not investigate it further, but i think it has to do with WeakReferences used inside Bindings.bindContentBidirectional.

tobiasdiez · 2019-09-09T10:37:55Z

Thanks! I think it is fine if we add the wrappers only when they are really necessary. So this PR is ready from your side?

r0light · 2019-09-09T11:26:22Z

Thanks! I think it is fine if we add the wrappers only when they are really necessary. So this PR is ready from your side?

Alright, not quite, i want have another look at it and also add some comments to the new classes.

r0light · 2019-09-09T12:04:25Z

Ok, from my side this would be ready to merge.
@tobiasdiez thanks for the help and feedback for this!

tobiasdiez

We have to thank you for your contribution. Code looks good to me!

Siedlerchr

Thanks very much for this contribution!

Siedlerchr · 2019-09-09T16:50:39Z

I guess #5010 is obsolete now?

30fb68e Create BJEDIS-ABNT-Number (#5255) aafb868 Update geochimica-et-cosmochimica-acta.csl (#5321) 60ba25f british-journal-of-anaesthesia.csl: add comma delimiter between non-sequential citations eg. 1 4 7-9 -> 1, 4, 7-9 (#5313) 67e6564 Reindent/reorder (#5318) c0d2a39 Ruby 3.0.0 (#5309) 76d60ff Update harvard-anglia-ruskin-university.csl (#5310) bc18ac9 Create journal-for-the-study-of-the-new-testament.csl (#5312) aff602c Update journal-of-food-protection.csl (#5315) 4503826 Update muscle-and-nerve.csl (#5317) 3bed58e constant redefinition 4d718a0 update documentaiton link fa99e2f add comma delimiter between succesive numbers d396f8b Allow privileged testing of PRs (#5307) 43b22c7 Update masarykova-univerzita-pravnicka-fakulta.csl, pravnik.csl, iso690-full-note-cs.csl (#5308) 8a31c1e Update copernicus-publications.csl (#5303) 96760bb Update anabases.csl (#5304) 744de6d removed locale (#5300) 7eb0d60 Update aviation-space-and-environmental-medicine.csl (#5297) 2769970 Update ruhr-universitat-bochum-lehrstuhl-fur-industrial-sales-and-ser… (#5298) 51e3f4c Update harvard-university-of-bath.csl (#5299) 5fce84f Create cns-spectrums.csl (#5290) bb8082c Create journal-of-surgical-oncology.csl (#5259) 90c13ae Update ruhr-universitat-bochum-lehrstuhl-fur-industrial-sales-and-ser… (#5288) 4bab1ad Update early-christianity.csl (#5289) 636ba78 Update tatup-zeitschrift-fur-technikfolgenabschatzung-in-theorie-und-… (#5291) b7cc511 Create biotechnologia.csl (#5292) 5bab881 Update journal-of-orthopaedic-trauma.csl (#5287) 5943413 Fix locales (#5285) 302bd65 Update universite-du-quebec-a-montreal-departement-dhistoire.csl (#5286) 860ae48 Add Haaga-Helia University of Applied Sciences Harvard style (#5282) c1c27de Localize Metropolia style title (#5283) 508da89 Fix presentation for Methods of Information in Medicine (#5284) 53e1d0b Create geschichte-und-gesellschaft.csl (#5216) d7ed0cb Create universite-de-geneve-departement-de-francais-moderne.csl (#5212) 80c404b Update journal-of-orthopaedic-trauma.csl (#5281) 20c143a Adding publishers' names (#5280) 6e5cd59 Update sodertorns-hogskola-oxford.csl (#5279) 52f2621 dollar-brace a260294 Create journal-of-microbiology-and-biotechnology.csl (#5277) 1fc979e Create qeios.csl (#5261) 86347b7 GH does this for us -- again, sorry guys b649589 Create experimental-biology-and-medicine.csl (#5276) 12ae0b1 Revert "tell sheldon about the job state" bdcae89 tell sheldon about the job state 1240067 Add Vegetation classification and Survey (#5271) 6f398f0 Major update to Gallia.csl (#5269) 2a74b2c Update filters.yaml (#5273) 20046d2 Update spec_helper.rb (#5272) 2ee0dd8 Create the-sociological-review.csl (#5260) 5b8d09c move filters to inert file to pacify Sheldon (#5268) e5f3315 Localize more language descriptors in style titles (#5270) bfd2942 Localize more language descriptors in style titles (#5267) 35e276f Fix variable used for the label after indication of number of pages (#5240) 60f6371 Create Universidade-do-Estado-do-Rio-de-Janeiro.csl (#5247) d8cc2ae Create the-journal-of-the-acoustical-society-of-america-numeric.csl (#5256) 92259c1 Create journal-of-financial-and-quantitative-analysis.csl (#5264) 6ba8aab Create journal-of-vestibular-research.csl (#5258) 0c88f41 Update european-journal-of-international-law.csl (#5265) cff5abc Put language descriptor within parentheses 4a62709 Update monash-university-harvard.csl (#5253) 64fd1aa Localize more language descriptors in style titles (#5262) f6519cb Localize more language descriptors in style titles (#5257) 170ccae tiny fixes for universitat-basel-iberoromanistik.csl (#5254) b7284c9 Localize more language descriptors in style titles (#5252) f4ef858 Add "Baishideng Publishing Group" dependents (#5251) 266e7c3 Make world-journal-of-hepatology.csl to bpg.csl parent (#5243) 9129098 fix small formatting issues for mclc.csl (#5229) 5d9560b Create crispr-journal.csl (#5249) a217299 Change "Czech" to "Čeština" in titles (#5248) 4fef39a Create journal-of-open-research-software.csl (#5245) 2bff1a6 Change "Dutch" to "Nederlands" in titles (#5242) f28da34 Update spec_helper.rb (#5246) e0e977c Move content from wiki pages to markdown files (#5194) 018304c Update universite-de-montreal-apa.csl (#5239) 3b83e5c Create sodertorns-hogskola-oxford.csl (#5234) 1335378 Stop notifying 8827 port on Zotero servers (#5237) f079b2a Update author-year disambiguation (#5238) 60bb0c9 Update technische-universitat-dresden-medizin.csl (#5236) e374657 Create Leidraad voor juridische auteurs 2019 (Dutch) (#5223) 0450d89 Add new style for U of Mannheim, Germanistische Linguistik (#5228) 81f0689 Create health-sports-rehabilitation-medicine.csl (#5233) c152a44 Update Gemfile.lock (#5235) 748e1eb Update geochimica-et-cosmochimica-acta.csl (#5231) 06b9ce8 Update zeitschrift-fur-theologie-und-philosophie.csl (#5230) e747cb1 haute-ecole-de-gestion-de-geneve: Make polyglot & et al changes 4cfedb7 Create universite-de-sherbrooke-histoire.csl (#5210) a96a61e Update journal-of-glaciology.csl (#5222) c6a94c9 Add Journal of Human Rights (#5227) c5c9c5f Update ruhr-universitat-bochum-lehrstuhl-fur-industrial-sales-and-ser… (#5214) ffb7aa6 Create comparativ.csl (#5215) e07329a Update lancaster-university-harvard.csl (#5220) c075d41 Update mimesis-edizioni.csl (#5219) 502970a Removed space in year only citation (#5218) 13e8c6b Update acta-scientiae-veterinariae.csl (#5209) 0699da6 Remake mammallia.csl for Oct/2020 guidelines. (#5207) b2dd3fd Update journal-of-international-business-studies.csl (#5217) dd52bfe Update quaternaire.csl (#5199) ccb1b0d rebuild webpage and article-journal citations in journal-of-forensic-sciences.csl (#5203) f02f4fb Create pedosphere.csl (#5196) 70dd87a Create open-gender-journal.csl (#5198) d272998 Create the-quarterly-journal-of-economics.csl (#5197) d27cab3 fix locale issues, add cite-locator (#5206) git-subtree-dir: buildres/csl/csl-styles git-subtree-split: 30fb68e

r0light added 2 commits September 8, 2019 22:52

added uithreadaware classes

8ba94d2

Merge branch 'master' into ensurejavafxthreadexecution

c902c1f

r0light changed the title ~~[WIP] Wrapper Classes for UI Lists etc that ensure updates on JavaFX thread~~ [WIP] Wrapper classes for UI Lists etc that ensure updates on JavaFX thread Sep 8, 2019

r0light mentioned this pull request Sep 8, 2019

Exception after cleanup entries dialog with rename PDF option enabled #5108

Closed

tobiasdiez requested changes Sep 8, 2019

View reviewed changes

r0light added 2 commits September 9, 2019 09:15

UiThreadHelper package-private and usage of UiThreadListDecorator onl…

96b9aa5

…y in View not in Model

added UiThreadStringProperty to be used in PersonsEditor

3001c84

tobiasdiez reviewed Sep 9, 2019

View reviewed changes

added comments

4592dca

r0light changed the title ~~[WIP] Wrapper classes for UI Lists etc that ensure updates on JavaFX thread~~ Wrapper classes for UI Lists etc that ensure updates on JavaFX thread Sep 9, 2019

tobiasdiez approved these changes Sep 9, 2019

View reviewed changes

Siedlerchr approved these changes Sep 9, 2019

View reviewed changes

Siedlerchr merged commit fc4201d into JabRef:master Sep 9, 2019

Siedlerchr mentioned this pull request Feb 3, 2020

Not on Fx Thread exception when editing keywords #5916

Closed

Siedlerchr mentioned this pull request Feb 20, 2020

Find and Replace raises IllegalStateException #5658

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wrapper classes for UI Lists etc that ensure updates on JavaFX thread #5289

Wrapper classes for UI Lists etc that ensure updates on JavaFX thread #5289

r0light commented Sep 8, 2019 •

edited

Loading

tobiasdiez left a comment

tobiasdiez Sep 8, 2019

r0light Sep 9, 2019

tobiasdiez Sep 8, 2019

r0light Sep 9, 2019

r0light commented Sep 9, 2019

tobiasdiez Sep 9, 2019

r0light Sep 9, 2019

tobiasdiez commented Sep 9, 2019

r0light commented Sep 9, 2019

r0light commented Sep 9, 2019

tobiasdiez left a comment

Siedlerchr left a comment

Siedlerchr commented Sep 9, 2019


		import javafx.application.Platform;

		public class UiThreadHelper {

Wrapper classes for UI Lists etc that ensure updates on JavaFX thread #5289

Wrapper classes for UI Lists etc that ensure updates on JavaFX thread #5289

Conversation

r0light commented Sep 8, 2019 • edited Loading

tobiasdiez left a comment

Choose a reason for hiding this comment

tobiasdiez Sep 8, 2019

Choose a reason for hiding this comment

r0light Sep 9, 2019

Choose a reason for hiding this comment

tobiasdiez Sep 8, 2019

Choose a reason for hiding this comment

r0light Sep 9, 2019

Choose a reason for hiding this comment

r0light commented Sep 9, 2019

tobiasdiez Sep 9, 2019

Choose a reason for hiding this comment

r0light Sep 9, 2019

Choose a reason for hiding this comment

tobiasdiez commented Sep 9, 2019

r0light commented Sep 9, 2019

r0light commented Sep 9, 2019

tobiasdiez left a comment

Choose a reason for hiding this comment

Siedlerchr left a comment

Choose a reason for hiding this comment

Siedlerchr commented Sep 9, 2019

r0light commented Sep 8, 2019 •

edited

Loading