[bisq-network/bisq] Added button to export voting history into JSON (#2504)

Manfred Karrer notifications at github.com
Wed Mar 6 17:37:51 UTC 2019


ManfredKarrer requested changes on this pull request.

Thanks for the fast implementation! Looks basically very good, just a few Nits...

> @@ -29,4 +31,7 @@
     <columnConstraints>
         <ColumnConstraints percentWidth="100"/>
     </columnConstraints>
+    <HBox GridPane.rowIndex="8" GridPane.columnSpan="3" alignment="BOTTOM_RIGHT">

We do not use FXML anymore beside of the containers (one day we want to get rid completely of it) as it has bad performance and is too limited for more dynamic content. There are still relicts but new code we want to have in Java only. See FormBuilder as out Util for UI components.

> @@ -105,6 +112,7 @@
     private final Preferences preferences;
     private final BsqFormatter bsqFormatter;
     private final Navigation navigation;
+    public AutoTooltipButton exportButton;

Would be private once removed from FXML

>          selectedVoteResultListItemListener = (observable, oldValue, newValue) -> onResultsListItemSelected(newValue);
 
         createCyclesTable();
+        exportButton.updateText(Res.get("shared.exportJSON"));

Here would be the code or creating the Button. 
E.g. `button = FormBuilder.addButtonAfterGroup( gridPane,  rowIndex,  title);`

> @@ -172,6 +182,57 @@ protected void activate() {
         cyclesTableView.getSelectionModel().selectedItemProperty().addListener(selectedVoteResultListItemListener);
 
         fillCycleList();
+        exportButton.setOnAction(event -> {

I would prefer to delegate that to a method to keep the activate() method short and easy to understand.
E.g. `exportButton.setOnAction(event -> doExportToJson()`

> @@ -268,6 +275,21 @@ public static void importAccounts(User user, String fileName, Preferences prefer
         }
     }
 
+    public static void exportJSON(String fileName, JsonElement data, Stage stage) {

Is there a pretty print feature in the json framework? Would be nice to have, but if not no worry.

> @@ -172,6 +182,57 @@ protected void activate() {
         cyclesTableView.getSelectionModel().selectedItemProperty().addListener(selectedVoteResultListItemListener);
 
         fillCycleList();
+        exportButton.setOnAction(event -> {

Could you add all fields (if missing, havve not checked all) from the domain objects which are currently not displayed in the UI.
We want to add popups for that anyway and for the json I think its good to have the complete domain data.

The CompensationProposal is missing `requestedBsq` and `bsqAddress`. As said the UI does not show all that yet but would be good to have that in the json.


> @@ -172,6 +182,57 @@ protected void activate() {
         cyclesTableView.getSelectionModel().selectedItemProperty().addListener(selectedVoteResultListItemListener);
 
         fillCycleList();
+        exportButton.setOnAction(event -> {

For Proposal there are a few subclasses with diff. fields...

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/2504#pullrequestreview-211366119
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20190306/b7afd1da/attachment-0001.html>


More information about the bisq-github mailing list