nics: Reimplement dialogs with new framework #2425

mvollmer · 2025-11-27T08:01:11Z

And introduce the framework at the same time.

Some noteworthy things:

It might be hard to see from the diff, but: the rewrite was very straightforward and it's nice that the "hairy" parts have disappeared, especially the untypable onValuesChanged methods. The code has become much less strongly coupled by that. Reusing components like NetworkModelRow is now feasible and doesn't expect the parent component to have a specific state structure.
This is as far as I think I want to go in this PR.

There is more to be done to make them "state of the art", but that requires actual changes to behavior. For example, the "Mac" text input in the Edit dialog could be a clean DialogTextInput, but it is sometimes disabled with a funny tooltip on the whole text input. I don't think this is a "best practice", it looks funny, I haven't seen it anywhere else, and I think we should reduce the use of tooltips in general. So instead of making DialogTextInput configurable for all these one-off "experiences", I think we should change the dialog to use a helper text to explain why the input is disabled. Likewise, I think we should change how the dialog handles types that have no sources. Etc. I'll continue in nics: Improve the dialog experience with new framework components #2435.

Here is the 1000 meter view of the framework API:

Define a new type for the values worked on by the dialog:

interface MyValues {
   name: string;
   flag: boolean;
}

Get a DialogState<MyValues> object with the initial values:

const dlg = useDialogState({ name: "Bob", flag: false });

Create a React modal thing as usual, but use DialogInputText etc in the body:

<ModalBody>
  <DialogErrorMessage dialog={dlg} />
  <Form>
    <DialogTextInput label="Name" value={dlg.value("name")} />
    <DialogCheckbox label="Flag" value={dlg.value("flag")} />
  </Form>
</ModalBody>

Use DialogActionButton in the footer:

<ModalFooter>
  <DialogActionButton dialog={dlg} action={some_action}>Apply</DialogActionButton>
  <DialogCancelButton dialog={dlg} />
</ModalFooter>

The action function gets the values and can throw errors:

async function some_action(values: MyValues) {
    ....
}

If you want to do input validation, write a function for that and pass it to use_DialogState. Don't worry too much why it needs to be written like this.

function validate() {
  dlg.value("name").validate(v => {
     if (v == "Bob")
        return _("No Bobs");
   });
}

src/components/vm/nics/nicAdd.tsx

cockpituous

There are more than 10 code coverage comments, see the full report here.

cockpituous

There are more than 10 code coverage comments, see the full report here.

cockpituous

There are more than 10 code coverage comments, see the full report here.

cockpituous

There are more than 10 code coverage comments, see the full report here.

mvollmer · 2025-11-27T14:40:08Z

Things that stick out in the dialogs that I would like to improve:

The excessive ceremony for giving things IDs.
The fact that NetworkTypeAndSource takes three values instead of just one, and thus doesn't follow the usual pattern.

The IDs are used by the tests but also to associate labels for input elements. Clicking the FormGroup label will focus the corresponding text input, and clicking a label for a checkbox should toggle that checkbox.

Using IDs for each and every thing in a dialog feels too much, but the alternative would be to use "data-ouia" things or similar, and that isn't really any better, is it?

So let's have a method for generating IDs for dialog values, and some Python helpers to work with them from the tests.

cockpituous

There are more than 10 code coverage comments, see the full report here.

cockpituous

There are more than 10 code coverage comments, see the full report here.

cockpituous

There are more than 10 code coverage comments, see the full report here.

cockpituous

There are more than 10 code coverage comments, see the full report here.

cockpituous

There are more than 10 code coverage comments, see the full report here.

To have only one dialog value instead of three (plus assorted props). The goal is to put the initialization knowledge in the component itself, and to not have any extra arguments for the validation function. This gives this component the "standard" API.

cockpituous

There are more than 10 code coverage comments, see the full report here.

cockpituous · 2025-12-16T08:05:49Z