-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
[WIP] Convert Exporter Customization Dialog to javafx #4394
Changes from 3 commits
f120b41
12e3e7d
ec364ad
f5c89fa
6773b22
1f318f0
1a0b760
5c65b57
2de4124
a869aa2
9ae84c0
e992c5d
3a4baa3
f63d43c
1f094d6
83b7dac
99d7041
0479893
547898c
e2ae2c9
3be5672
debd2bc
7141a46
889489b
1fb234d
e17d0da
73e683a
19720cc
4e8c8df
3618f83
51a726e
1821da3
d339f78
eea34ed
d36eec2
294a367
4269ca0
25db576
3b54bc8
84f293e
00bac82
b26d592
b681ae8
be47855
9aa0887
d7277e8
7087cfb
1daf7c4
83d490a
17d20cc
40eed27
379f201
4679150
0053abf
9442025
de3bf63
19d8c62
df42676
f31bc51
d95a1aa
80ffa33
5f56c25
f1d920e
f22e97a
252ecbb
ccdbf1f
aba31a6
8cd7ff3
3e09a7b
3051022
0ef3061
7bf6dd6
93838de
a79d037
70479cb
bf22adf
c2b3489
a364e1d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
package org.jabref.gui.exporter; | ||
|
||
import java.util.List; | ||
import java.util.Optional; | ||
|
||
import javax.inject.Inject; | ||
|
||
import javafx.beans.property.SimpleListProperty; | ||
import javafx.collections.FXCollections; | ||
|
||
import org.jabref.gui.DialogService; | ||
import org.jabref.gui.util.BaseDialog; | ||
import org.jabref.logic.exporter.TemplateExporter; | ||
import org.jabref.preferences.CustomExportList; | ||
import org.jabref.preferences.JabRefPreferences; | ||
import org.jabref.preferences.PreferencesService; | ||
|
||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
public class ExportCustomizationDialogViewModel extends BaseDialog<Void> { | ||
|
||
//The class vars might need to be reordered | ||
|
||
private final SimpleListProperty<ExporterViewModel> exporters = new SimpleListProperty<>(FXCollections.observableArrayList()); | ||
private final int size; //final? Or you don't need this and just use a a while loop | ||
|
||
//Indices within which export format information is stored within JabRefPreferences | ||
private static final int EXPORTER_NAME_INDEX = 0; | ||
private static final int EXPORTER_FILENAME_INDEX = 1; | ||
private static final int EXPORTER_EXTENSION_INDEX = 2; | ||
|
||
private final PreferencesService preferences; | ||
private final DialogService dialogService; | ||
|
||
private static final Logger LOGGER = LoggerFactory.getLogger(CustomExportList.class); | ||
|
||
//Other variable declarations here | ||
|
||
//Also write tests for all of this | ||
|
||
public ExportCustomizationDialogViewModel(DialogService dialogService) { | ||
this.dialogService = dialogService; | ||
|
||
int i = 0; | ||
//raname var "s"? | ||
List<String> s; | ||
while (!((s = preferences.getStringList(JabRefPreferences.CUSTOM_EXPORT_FORMAT + i)).isEmpty())) { | ||
//Shuold createFormat be in logic? Check how it works in CustomExportList | ||
Optional<ExporterViewModel> format = createFormat(s.get(EXPORTER_NAME_INDEX), s.get(EXPORTER_FILENAME_INDEX), s.get(EXPORTER_EXTENSION_INDEX), layoutPreferences, savePreferences); | ||
if (format.isPresent()) { | ||
exporters.add(format.get()); //put was changed to add becuase we are not dealing with Map as in CustomExpoertList | ||
} else { | ||
String customExportFormat = preferences.get(JabRefPreferences.CUSTOM_EXPORT_FORMAT + i); | ||
LOGGER.error("Error initializing custom export format from string " + customExportFormat); | ||
} | ||
i++; | ||
} | ||
//ExporterViewModel will be organized as a singular version of what now is CustomExportDialog, which | ||
//currently stores all the exporters in a class var. Each ViewModel will have one exporter and associated data, and | ||
//the class var exporters will be a list of them | ||
//You will have to write properites into ExpoerterViewModel that get all the relevant information about the exporter | ||
//in addition to a property for the exporter itself | ||
|
||
} | ||
|
||
//possibly not necessary, and if so getSortedList will have to return the correct type, not Eventlist<List<String>> | ||
public List<ExporterViewModel> loadExporters() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You might don't need the gui-wrapper There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have put the data in the GUI wrapper There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can also simply use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay. I've finished, but not tested, most of the |
||
return preferences.customExports.getSortedList(); //As of now getSortedList refers to EventList<List<String>> | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,4 +35,8 @@ public interface PreferencesService { | |
|
||
public void updateEntryEditorTabList(); | ||
|
||
List<String> getStringList(String key); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please no generic methods here, create specific ones for the cases you need, e.g. get CustomExporters or getExporterormat... Or you create CustomExporter preferences which has all the needed information inside. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Swing version has a class CustomExportList which holds a list of custom exporters and associated data. I was going to move some of the code here into the ExporterViewModel and other code into the ExporterCustomizationDialogViewModel. Do you recommend keeping the CustomExportList as it is? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The PreferencesService should only contain usecase-specific preference methods and the caller shouldn't care about how these information are stored in the preferences. So, here, the method would look like Thus, most of the code in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't the methods in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I don't really understand what you mean. The PreferencesService should provide an interface to the preferences that hide the exact semantics of how the object is serialized. So abstractly, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Quick question: Do you recommend writing tests for the view model? The dialog is simple, but not as simple as the previous one I wrote. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not a big fan of writing tests for the sake of it. My strategy is usually to program everything and when I discover a problem in the logic during testing, I'll write a test for it. Hence, if the dialog is simple enough that you don't make any errors, then you don't need to write tests ;-). |
||
|
||
String get(String key); | ||
|
||
} |
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.
The
View
/Dialog class should derive fromBaseDialog
and not the view model.