nics: Improve the dialog experience with new framework components #2435

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.

Sign up for GitHub

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

Jump to bottom

Draft

mvollmer wants to merge 3 commits into cockpit-project:main from mvollmer:nics-dialogs-nip-and-tuck

Member

mvollmer commented Dec 2, 2025 •

edited

Loading

nics: Reimplement dialogs with new framework #2425
Add link to "direct" explanation
Use RadioSelect etc dialog helpers in tests
Make sure that at least one interface type is always available
Document the update_func when creating value handles.
test for "create default network"
make "create default" work on "arch". Just include the definition in our code?

This adds the obvious porcelain components for checkboxes, dropdown selects and radio buttons and uses them in the "nics" dialogs. At the same time, the dialog experience is improved in a number of ways:

Instead of being read-only with a tooltip, the mac field is now disabled with a helper text. The readonly state is rare and communicates less well than the disabled state. Tooltips (that open on hover) are not mobile friendly and generally awkward. There is enough space in the dialog for a inline explanation.
The "interface type" is selected with radio buttons instead of a dropdown. There are only three types and there is enough space to show them all. Instead of a info popover with questionably typography and content, the choices now have brief helper text to explain their purpose.
When a interface type has no sources, the type is disabled, instead of keeping it enabled and showing an empty list of sources, which would be a dead-end.
When editing a interface, the current source is always in the "Source" dropdown, even if it doesn't currently exist. Consequently, the current type is always enabled.
When adding a interface and there are no virtual networks, the "Source" dropdown offers to to create the "default" network (which really should always exist anyway). This means that the "network" type is always enabled.

mvollmer added the blocked label

mvollmer changed the title ~~Nics dialogs nip and tuck~~ nics: Improve the dialog experience with new framework components

cockpituous reviewed

View reviewed changes

Contributor

cockpituous left a comment

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

mvollmer force-pushed the nics-dialogs-nip-and-tuck branch from 26c86f5 to 5168d18 Compare

December 2, 2025 16:20

cockpituous reviewed

View reviewed changes

Contributor

cockpituous left a comment

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

mvollmer force-pushed the nics-dialogs-nip-and-tuck branch from 5168d18 to dc7d51d Compare

December 2, 2025 18:18

cockpituous reviewed

View reviewed changes

Contributor

cockpituous left a comment

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

mvollmer mentioned this pull request

nics: Reimplement dialogs with new framework #2425

Open

1 task

mvollmer force-pushed the nics-dialogs-nip-and-tuck branch from dc7d51d to ea2d88b Compare

December 3, 2025 14:31

cockpituous reviewed

View reviewed changes

Contributor

cockpituous left a comment

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

mvollmer force-pushed the nics-dialogs-nip-and-tuck branch 2 times, most recently from 03b053c to 9e650a2 Compare

December 3, 2025 15:11

cockpituous reviewed

View reviewed changes

Contributor

cockpituous left a comment

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

mvollmer force-pushed the nics-dialogs-nip-and-tuck branch from 9e650a2 to e22b04b Compare

December 4, 2025 08:15

cockpituous reviewed

View reviewed changes

Contributor

cockpituous left a comment

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

mvollmer force-pushed the nics-dialogs-nip-and-tuck branch from e22b04b to ec962d2 Compare

December 4, 2025 10:49

cockpituous reviewed

View reviewed changes

Contributor

cockpituous left a comment

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

mvollmer force-pushed the nics-dialogs-nip-and-tuck branch from ec962d2 to dfd70a6 Compare

December 8, 2025 13:34

cockpituous reviewed

View reviewed changes

Contributor

cockpituous left a comment

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

mvollmer force-pushed the nics-dialogs-nip-and-tuck branch from dfd70a6 to 4e63f3e Compare

December 9, 2025 14:24

cockpituous reviewed

View reviewed changes

Contributor

cockpituous left a comment

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

mvollmer force-pushed the nics-dialogs-nip-and-tuck branch 2 times, most recently from 520176e to dd682c7 Compare

December 9, 2025 15:28

cockpituous reviewed

View reviewed changes

Contributor

cockpituous left a comment

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

mvollmer mentioned this pull request

disks: Redo "Add disk" and "Insert media" dialog with new framework #2449

Open

1 task

mvollmer force-pushed the nics-dialogs-nip-and-tuck branch from dd682c7 to 474d21e Compare

December 11, 2025 13:09

cockpituous reviewed

View reviewed changes

Contributor

cockpituous left a comment

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

mvollmer force-pushed the nics-dialogs-nip-and-tuck branch from 474d21e to 336a232 Compare

December 15, 2025 09:14

cockpituous reviewed

View reviewed changes

Contributor

cockpituous left a comment

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

mvollmer force-pushed the nics-dialogs-nip-and-tuck branch from 336a232 to 1d0f9e3 Compare

December 15, 2025 17:15

cockpituous reviewed

View reviewed changes

Contributor

cockpituous left a comment

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

mvollmer force-pushed the nics-dialogs-nip-and-tuck branch from 1d0f9e3 to 3b71159 Compare

December 16, 2025 07:55

cockpituous reviewed

View reviewed changes

Contributor

cockpituous left a comment

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

mvollmer force-pushed the nics-dialogs-nip-and-tuck branch from 3b71159 to 6408b0c Compare

December 16, 2025 12:05

cockpituous reviewed

View reviewed changes

Contributor

cockpituous left a comment

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

mvollmer force-pushed the nics-dialogs-nip-and-tuck branch from 6408b0c to 4e45c3b Compare

December 17, 2025 17:13

cockpituous reviewed

View reviewed changes

Contributor

cockpituous left a comment

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

mvollmer force-pushed the nics-dialogs-nip-and-tuck branch from 4e45c3b to 0ef9729 Compare

December 17, 2025 18:35

cockpituous reviewed

View reviewed changes

Contributor

cockpituous left a comment

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

mvollmer force-pushed the nics-dialogs-nip-and-tuck branch from 0ef9729 to d596cf6 Compare

December 22, 2025 08:14

cockpituous reviewed

View reviewed changes

Contributor

cockpituous left a comment

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

src/components/vm/nics/nicAdd.tsx

    
                              name="mac-setting"

                              isChecked={!v_set.get()}

                              label={_("Generate automatically")}

                              onChange={() => v_set.set(false)}

Contributor

cockpituous Dec 22, 2025

This added line is not executed by any test. Details

src/components/vm/nics/nicAdd.tsx

    
              function validate_NetworkMacRow(value: DialogValue<NetworkMacValue>) {

                  value.validate(v => {

                      if (v.set && v.val === "")

                          return _("Network MAC can not be empty.");

Contributor

cockpituous Dec 22, 2025

This added line is not executed by any test. Details

src/components/vm/nics/nicAdd.tsx

    
                                  mac: (

                                      values.mac.set

                                          ? values.mac.val

                                          : getRandomMac(await appState.getVms())

Contributor

cockpituous Dec 22, 2025

This added line is not executed by any test. Details

src/components/vm/nics/nicAdd.tsx

    
                  const defaultBody = (

                      <>

                          <Form onSubmit={e => e.preventDefault()} isHorizontal>

Contributor

cockpituous Dec 22, 2025

This added line is not executed by any test. Details

src/components/vm/nics/nicBody.tsx

    
                  if (osTypeArch == 'ppc64' && osTypeMachine == 'pseries')

                      availableModelTypes.push({ name: 'spapr-vlan' });

                      availableModelTypes.push({ value: 'spapr-vlan', label: 'spapr-vlan' });

Contributor

cockpituous Dec 22, 2025

This added line is not executed by any test. Details

src/components/vm/nics/nicBody.tsx

    
                  } else {

                      // User session

                      if (dialogValues.availableSources.network.length > 0) {

                      if (availableSources.network && availableSources.network.length > 0) {

Contributor

cockpituous Dec 22, 2025

This added line is not executed by any test. Details

src/components/vm/nics/nicBody.tsx

Comment on lines +131 to +132

    
                                  detailHeadline: _("Provides a virtual LAN with NAT to the outside world."),

                                  disabled: false,

Contributor

cockpituous Dec 22, 2025

These 2 added lines are not executed by any test. Details

src/components/vm/nics/nicBody.tsx

    
                      // matter what.

                      if (network.type in _availableSourcesForType) {

                          const avail = _availableSourcesForType[network.type];

                          const src = getNetworkSource(network) || "";

Contributor

cockpituous Dec 22, 2025

This added line is not executed by any test. Details

src/components/vm/nics/nicBody.tsx

    
                          if (!avail.includes(src))

                              avail.push(src);

                      }

                  } else if (vm.connectionName == "system" && _availableSourcesForType.network.length == 0) {

Contributor

cockpituous Dec 22, 2025

This added line is not executed by any test. Details

src/components/vm/nics/nicBody.tsx

    
                  if (network) {

                      type = network.type;

                      source = getNetworkSource(network) || "";

                      mode = (type == "direct" ? (network.source.mode || "") : "bridge");

Contributor

cockpituous Dec 22, 2025

This added line is not executed by any test. Details

mvollmer mentioned this pull request

lib: Introduce the Dialog Implementation Convenience Kit cockpit-project/cockpit#22700

Open

mvollmer added 3 commits

January 8, 2026 10:35


          XXX - get dialog framework

345e2b1


          nics: Reimplement dialogs with new framework

247f66a


          nics: Improve dialogs with new framework components

000eda6

 - Use DialogRadioSelect etc as appropriate.

 - Change TypeAndSource to disable types without sources instead of
   disabling the sources selector.

 - When editing a interface, always include the current source in the
   choices, even if it doesn't actually exist.

 - When adding, re-create "default" network when none other exists to
   always have a path through the dialog.

mvollmer force-pushed the nics-dialogs-nip-and-tuck branch from d596cf6 to 000eda6 Compare

January 8, 2026 09:37

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels