-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Tries to address the refresh problem of the table in the dialog "Manage external file types" (issue 5902) #5907
Conversation
…am"/"custom program" fixed: it now gets updated properly (binding added); - "Edit file type dialog": correct value get loaded now for the input field "Name" - when storing an updated file type: no differentiation will be made any more whether program runs on Windows or somewhere else - quick fix for pending issue: editing an ExternalFileType works now (still room for improvement); visual bug concerning "lazy" update of table could be a Linux issue, since other tables are affected as well; so basically - except the "lazy" update thing -- everything works concerning external file types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small comments.
@Siedlerchr @tobiasdiez what's your take on this?
return name; | ||
} | ||
|
||
@Override | ||
public String getExtension() { | ||
public String getNameAsString() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some code uses .getValue()
some ...AsString()
I would always stick to .getValue()
} | ||
|
||
public void addNewType() { | ||
CustomExternalFileType type = new CustomExternalFileType("", "", "", "", "new", IconTheme.JabRefIcons.FILE); | ||
CustomExternalFileType type = new CustomExternalFileType(new SimpleStringProperty(""), new SimpleStringProperty(""), new SimpleStringProperty(""), new SimpleStringProperty(""), new SimpleStringProperty("new"), IconTheme.JabRefIcons.FILE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't a string be passed and converted in CustomExternalFileTypes
internally?
|
||
String getExtension(); | ||
StringProperty getExtension(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is the StringProperty
used? I always saw getValue()
. Did I miss something?
I don't this is the right approach with passing the StringProperty down, I would rather create a ViewModel for the table like I did in the Custom entry types dialog and not modifying the original implementation of the model type. |
I would recommend, that @Siedlerchr fixes this issue as needed. He has (positive) experience concerning this matter. :) |
@systemoperator This is your chance to learn about MVVM. See https://devdocs.jabref.org/javafx#architecture-model-view-controller-viewmodel-mv-c-vm for details. I hope, you will resume work here. |
b8ef7b7
to
21c6e5e
Compare
I agree with #5907 (comment). @systemoperator it would be nice if you could implement this! Thanks. |
I also looked into the original code, and I could also reproduce this under Windows and I guess it's due to the Singleton stuff updating. |
I see that MVVM is hard to implement. @Siedlerchr will take care later. We will close for now as we have to focus on the 5.1 release 🔥 |
76b4268 Style for Acta Physica Sinica (物理学报) (#6009) 604de6f MLA Publication Date update (#5927) 41ce2d4 Update netherlands-journal-of-geosciences-geologie-en-mijnbouw.csl (#6027) ad08f5d Adds space after title writing in ABNT style file (#6031) 0b5c1c2 Create CRCAO Light (#5935) 2f42b8c Create annals-of-laboratory-medicine.csl (#5959) 80a9506 Create annual-review-of-linguistics (#5992) 0fb9c40 Update the-journal-of-egyptian-archaeology.csl (#6028) 5ff53b1 Update guide-des-citations-references-et-abreviations-juridiques.csl (#6002) c1c0212 Update society-for-american-archaeology.csl (#5919) 129ef3c Update administrative-science-quarterly.csl (#5991) 8bc22bd Create multilingual-matters.csl (#5955) aca84fd Create expert-reviews-in-molecular-medicine.csl (#5961) 3c4ddc0 Create ethnobiology-letters.csl (#5986) c301e04 Update heidelberg-university-faculty-of-medicine.csl (#5932) a8c4396 Update tyndale-bulletin.csl (#5949) c18cbcf Bluebook hotfix d950b2b Create incontext-studies-in-translation-and-interculturalism.csl (#5907) 7cfc106 Bump nokogiri from 1.13.2 to 1.13.4 (#6016) 0baa43a Liebert update (#6026) 41ca2b3 Bluebook Add space for ibid (#6025) 6707a37 Update american-journal-of-botany.csl (#5954) 926fad5 Update boletin-de-pediatria.csl (#6024) git-subtree-dir: buildres/csl/csl-styles git-subtree-split: 76b4268
76b4268 Style for Acta Physica Sinica (物理学报) (JabRef#6009) 604de6f MLA Publication Date update (JabRef#5927) 41ce2d4 Update netherlands-journal-of-geosciences-geologie-en-mijnbouw.csl (JabRef#6027) ad08f5d Adds space after title writing in ABNT style file (JabRef#6031) 0b5c1c2 Create CRCAO Light (JabRef#5935) 2f42b8c Create annals-of-laboratory-medicine.csl (JabRef#5959) 80a9506 Create annual-review-of-linguistics (JabRef#5992) 0fb9c40 Update the-journal-of-egyptian-archaeology.csl (JabRef#6028) 5ff53b1 Update guide-des-citations-references-et-abreviations-juridiques.csl (JabRef#6002) c1c0212 Update society-for-american-archaeology.csl (JabRef#5919) 129ef3c Update administrative-science-quarterly.csl (JabRef#5991) 8bc22bd Create multilingual-matters.csl (JabRef#5955) aca84fd Create expert-reviews-in-molecular-medicine.csl (JabRef#5961) 3c4ddc0 Create ethnobiology-letters.csl (JabRef#5986) c301e04 Update heidelberg-university-faculty-of-medicine.csl (JabRef#5932) a8c4396 Update tyndale-bulletin.csl (JabRef#5949) c18cbcf Bluebook hotfix d950b2b Create incontext-studies-in-translation-and-interculturalism.csl (JabRef#5907) 7cfc106 Bump nokogiri from 1.13.2 to 1.13.4 (JabRef#6016) 0baa43a Liebert update (JabRef#6026) 41ca2b3 Bluebook Add space for ibid (JabRef#6025) 6707a37 Update american-journal-of-botany.csl (JabRef#5954) 926fad5 Update boletin-de-pediatria.csl (JabRef#6024) git-subtree-dir: buildres/csl/csl-styles git-subtree-split: 76b4268
References: #5902
Tries to address the visual issue, where the table in the dialog "Manage external file types" (Options > Manage external file types) is only refreshed, when an updated entry firstly gets scrolled out of the visible table area and then gets scrolled back in, so that the table entry is again visible, then also the values contained in this changed entry is actually shown updated.
Currently, it does not fix it, but maybe it is useful and/or should be merged anyway, since other JavaFX tables are implemented in the same way, by using the extractor property. You decide.