natcap / invest-workbench

An Electron application front-end for InVEST
Other
1 stars 5 forks source link

Define UI specifications for each invest model #31

Closed davemfish closed 3 years ago

davemfish commented 4 years ago

Each model can have a JSON specification to define the relationships among individual input fields in that model's setup. The design doc for this feature: https://docs.google.com/document/d/1VYOoHYXV7gIf2VQaiphC9ahr8zOtbeETQRESQtyvEmg/edit#heading=h.9r56n017eqt4

Things that can be controlled:

A few models already have specs (e.g. https://github.com/natcap/invest-gui/blob/master/ui_data/natcap.invest.ndr.ndr.json).

I can commit boilerplate specs for all other models, but then we'll need to populate those with values. These specs are not required for a functional application, but do provide a nicer user-experience.

davemfish commented 4 years ago
emlys commented 3 years ago

Just started on this but before I get too far, what would you think about refactoring the UI spec order option a bit? Right now the order is determined by a decimal number where the integer part is the section, and the decimal part is the place within that section. E.g. "order": 0.3 means the 3rd item in the 1st section (if I'm understanding correctly).

To me it might make more sense to have a separate "order" section in the top-level of the JSON. Something like:

{
    "order": [
        ["workspace_dir", "results_suffix"],
        ["lulc_cur_path", "carbon_pools_path"],
        ["calc_sequestration", "lulc_fut_path", "lulc_cur_year", "lulc_fut_year"],
        ["do_redd", "lulc_redd_path"],
        ["do_valuation", "price_per_metric_ton_of_c", "discount_rate", "rate_change"]
    ],
    "workspace_dir": {},
    "results_suffix": {},
    "lulc_cur_path": {},
    "calc_sequestration": {
        "ui_option": "disable",
        "ui_control": [
            "lulc_fut_year",
            "lulc_cur_year",
            "lulc_fut_path"
        ]
    },
    "do_redd": {
        "ui_control": [
            "lulc_redd_path"
        ]
    },
    "do_valuation": {
        "ui_control": [
            "rate_change",
            "discount_rate",
            "price_per_metric_ton_of_c"
        ]
    },
    "lulc_fut_path": {
        "ui_option": "disable"
    },
    "lulc_redd_path": {
        "ui_option": "disable"
    },
    "carbon_pools_path": {},
    "lulc_cur_year": {
        "ui_option": "disable"
    },
    "lulc_fut_year": {
        "ui_option": "disable"
    },
    "price_per_metric_ton_of_c": {
        "ui_option": "disable"
    },
    "discount_rate": {
        "ui_option": "disable"
    },
    "rate_change": {
        "ui_option": "disable"
    }
}

Benefits I see are:

I may be missing something though. Are there possible future options that are better represented with the numeric ordering? Would love to hear your thoughts @davemfish

davemfish commented 3 years ago

I think these are all great points! I'm guessing we'll end up with cleaner, simpler code in the form components as well if we do this. I'm all in favor. Like you say, it's probably wise to do the refactor first with the few UI specs that are already checked in, and developing against the tests in setuptab.test.js, and then get that PR'ed before diving back into adding more models. Would you like to work on that refactor, @emlys ?

I think the main requirements for this feature are the 4 bullet points from the opening post on this issue. Another requirement we've kept thus far is that all model forms should still work without a UI spec. But that requirement could be open for debate, once all the models have a spec file.

emlys commented 3 years ago

@davemfish happy to work on it! Thanks for the input :)

emlys commented 3 years ago

This is now being resolved in https://github.com/natcap/invest-workbench/pull/84

davemfish commented 3 years ago

And #84 is done!