Fix options databinding by adding elements if they are not in the initial map #3539
Conversation
Siedlerchr
changed the title
|
Can you please explain the reason for the bug? In general, the string converter should not modify the source or target map. |
|
The reason for the bug is that the fromString method does a lookup in the map with the current txt in the field. If you enter a non predefined String into it, the map lookup returns null because not found. |
| return getItemMap().getOrDefault(string, null); | ||
|
|
||
| if (getItemMap().get(string) == null) { | ||
| getItemMap().put(string, (T) string); |
There was a problem hiding this comment.
Ok thanks!
According to the documentation
a string converter property is provided to allow for developers to specify how to translate a users string into an object of type T, such that the value property may contain it. By default the converter simply returns the String input as the user typed it, which therefore assumes that the type of the editable ComboBox is String.
So you are right that the string converter should return something non-null even when the text is not recognized. However, there is no need to add it to the item map. Moreover, we need to handle the case when T is not a string type. Thus I would propose to create a new protected method that converts an unknown string to a T object. If not overridden, it should try to cast to T as in your code above (and throw an exception with details how to properly implement it if casting was not successful).
There was a problem hiding this comment.
I now added a method for checking.
We also need such kind for dates to allow custom values which we currently don't support. All input in the field will be cleared if not parseable
…ding * upstream/master: (29 commits) New translations JabRef_en.properties (Indonesian) Changed workflow because of Crowdin Update various junit plugins Update johnrengelman.shadow from 2.0.1 -> 2.0.2 update Junit5 from 5.0.2 -> 5.1.0-M1 Update build-scan from 1.10.3 -> 1.11 Update antlr4 from 4.7 -> 4.7.1 Update log4j from 2.9.1 -> 2.10.0 Update mockito-core from 2.12.0 -> 2.13.0 New translations JabRef_en.properties (Vietnamese) New translations JabRef_en.properties (Italian) New translations JabRef_en.properties (Indonesian) New translations JabRef_en.properties (Greek) New translations JabRef_en.properties (German) New translations JabRef_en.properties (French) New translations JabRef_en.properties (Dutch) New translations JabRef_en.properties (Danish) New translations JabRef_en.properties (Chinese Simplified) New translations JabRef_en.properties (Japanese) New translations JabRef_en.properties (Norwegian) ...
catch classCast Exceptions
Siedlerchr
added
the
status: ready-for-review
Fixes #3538
I dunno if this is the best idea, but it works...
gradle localizationUpdate?