[PATCH installer v4 28/30] tui: network: split advanced options dialog into tabs

[Christoph Heiss <c.heiss@proxmox.com>]

With the bond and VLAN options, the advanced network options dialog
stacks three sections, and both the pinning and the bond section list
every physical network interface. Servers with six or more ports are
not unusual, so the dialog needs a lot of scrolling to get through, in
particular on an 80x24 console.

Put each section on its own tab, as in the graphical installer, and let
each tab scroll on its own. If an input is invalid, switch to the tab
that contains it before showing the error, as it could otherwise be on
a tab that is not visible.

Originally-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v3 -> v4:
  * no changes

Changes v2 -> v3:
  * new patch

 proxmox-tui-installer/src/views/network.rs    | 85 +++++++++++++------
 .../src/views/tabbed_view.rs                  |  7 ++
 2 files changed, 66 insertions(+), 26 deletions(-)

diff --git a/proxmox-tui-installer/src/views/network.rs b/proxmox-tui-installer/src/views/network.rs
index ad5d1d9..af0528a 100644
--- a/proxmox-tui-installer/src/views/network.rs
+++ b/proxmox-tui-installer/src/views/network.rs
@@ -3,8 +3,8 @@ use cursive::{
     Cursive, View,
     view::{Nameable, Resizable, ViewWrapper},
     views::{
-        Button, Checkbox, Dialog, DummyView, EditView, LinearLayout, NamedView, Panel, ResizedView,
-        ScrollView, SelectView, TextView,
+        Button, Checkbox, Dialog, DummyView, EditView, LinearLayout, NamedView, PaddedView,
+        ResizedView, ScrollView, SelectView, TextView,
     },
 };
 use std::{
@@ -22,7 +22,7 @@ use proxmox_installer_types::answer::{NetworkBondOptions, NetworkVlanOptions};
 use proxmox_network_api::{BondXmitHashPolicy, LinuxBondMode};
 use proxmox_network_types::{MacAddress, fqdn::Fqdn, ip_address::Cidr};
 
-use super::{CheckboxGroup, CidrAddressEditView, FormView, IntegerEditView};
+use super::{CheckboxGroup, CidrAddressEditView, FormView, IntegerEditView, TabbedView};
 
 struct NetworkViewOptions {
     pinning_enabled: bool,
@@ -58,6 +58,12 @@ impl NetworkOptionsView {
     const INTERFACE_PINNING_VIEW_NAME: &str = "network-name-pinning-view";
     const BOND_OPTIONS_VIEW_NAME: &str = "network-bond-options-view";
     const VLAN_OPTIONS_VIEW_NAME: &str = "network-vlan-options-view";
+    const ADVANCED_OPTIONS_TABS_NAME: &str = "network-advanced-options-tabs";
+
+    // tab indices in the advanced options dialog
+    const INTERFACE_PINNING_TAB: usize = 0;
+    const BOND_OPTIONS_TAB: usize = 1;
+    const VLAN_OPTIONS_TAB: usize = 2;
 
     pub fn new(options: &NetworkOptions, network_info: &NetworkInfo) -> Self {
         let mut interfaces = network_info
@@ -226,23 +232,35 @@ impl NetworkOptionsView {
     }
 
     fn advanced_options_view(options_ref: NetworkViewOptionsRef) -> impl View {
-        let inner = ScrollView::new(
-            LinearLayout::vertical()
-                .child(Panel::new(
+        // with many interfaces, the pinning and bond pages can be taller than the screen
+        fn page(view: impl View) -> PaddedView<ScrollView<impl View>> {
+            PaddedView::lrtb(1, 1, 1, 0, ScrollView::new(view))
+        }
+
+        let tabs = TabbedView::new()
+            .tab(
+                "Interface Name Pinning",
+                page(
                     InterfacePinningOptionsView::new(options_ref.clone())
                         .with_name(Self::INTERFACE_PINNING_VIEW_NAME),
-                ))
-                .child(Panel::new(
+                ),
+            )
+            .tab(
+                "Management Bond",
+                page(
                     BondOptionsView::new(options_ref.clone())
                         .with_name(Self::BOND_OPTIONS_VIEW_NAME),
-                ))
-                .child(Panel::new(
+                ),
+            )
+            .tab(
+                "Management VLAN",
+                page(
                     VlanOptionsView::new(options_ref.clone())
                         .with_name(Self::VLAN_OPTIONS_VIEW_NAME),
-                )),
-        );
+                ),
+            );
 
-        Dialog::around(inner)
+        Dialog::around(tabs.with_name(Self::ADVANCED_OPTIONS_TABS_NAME))
             .title("Advanced Network Options")
             .button("Ok", {
                 move |siv| {
@@ -254,7 +272,12 @@ impl NetworkOptionsView {
                             options.bond_options = bond_opts;
                             options.vlan_options = vlan_opts;
                         }
-                        Err(err) => {
+                        Err((tab, err)) => {
+                            // the invalid input might be on a tab that is not shown currently
+                            siv.call_on_name(
+                                Self::ADVANCED_OPTIONS_TABS_NAME,
+                                |view: &mut TabbedView| view.set_current(tab),
+                            );
                             siv.add_layer(Dialog::info(err.to_string()));
                             return;
                         }
@@ -266,45 +289,55 @@ impl NetworkOptionsView {
             .max_size((80, 40))
     }
 
+    /// Returns the values of all advanced options, or the error together with the index of the
+    /// tab containing the invalid input.
     fn get_view_values(
         siv: &mut Cursive,
         options_ref: NetworkViewOptionsRef,
-    ) -> Result<(
-        NetworkInterfacePinningOptions,
-        NetworkBondOptions,
-        NetworkVlanOptions,
-    )> {
+    ) -> Result<
+        (
+            NetworkInterfacePinningOptions,
+            NetworkBondOptions,
+            NetworkVlanOptions,
+        ),
+        (usize, anyhow::Error),
+    > {
         let pin_opts = siv
             .call_on_name(
                 Self::INTERFACE_PINNING_VIEW_NAME,
                 InterfacePinningOptionsView::get_values,
             )
-            .unwrap_or_else(|| {
-                bail!("failed to retrieve network interface name pinning options")
-            })?;
+            .unwrap_or_else(|| bail!("failed to retrieve network interface name pinning options"))
+            .map_err(|err| (Self::INTERFACE_PINNING_TAB, err))?;
 
         let bond_opts = siv
             .call_on_name(Self::BOND_OPTIONS_VIEW_NAME, BondOptionsView::get_values)
-            .unwrap_or_else(|| bail!("failed to retrieve management network bond options"))?;
+            .unwrap_or_else(|| bail!("failed to retrieve management network bond options"))
+            .map_err(|err| (Self::BOND_OPTIONS_TAB, err))?;
 
         if options_ref
             .lock()
             .map(|opts| opts.bond_enabled)
             .unwrap_or(false)
         {
-            bond_opts.verify()?;
+            bond_opts
+                .verify()
+                .map_err(|err| (Self::BOND_OPTIONS_TAB, err))?;
         }
 
         let vlan_opts = siv
             .call_on_name(Self::VLAN_OPTIONS_VIEW_NAME, VlanOptionsView::get_values)
-            .unwrap_or_else(|| bail!("failed to retrieve management network VLAN options"))?;
+            .unwrap_or_else(|| bail!("failed to retrieve management network VLAN options"))
+            .map_err(|err| (Self::VLAN_OPTIONS_TAB, err))?;
 
         if options_ref
             .lock()
             .map(|opts| opts.vlan_enabled)
             .unwrap_or(false)
         {
-            vlan_opts.verify()?;
+            vlan_opts
+                .verify()
+                .map_err(|err| (Self::VLAN_OPTIONS_TAB, err))?;
         }
 
         Ok((pin_opts, bond_opts, vlan_opts))
diff --git a/proxmox-tui-installer/src/views/tabbed_view.rs b/proxmox-tui-installer/src/views/tabbed_view.rs
index 1331713..4c849ea 100644
--- a/proxmox-tui-installer/src/views/tabbed_view.rs
+++ b/proxmox-tui-installer/src/views/tabbed_view.rs
@@ -40,6 +40,13 @@ impl TabbedView {
         self
     }
 
+    /// Makes the tab at `index` the active one, if it exists.
+    pub fn set_current(&mut self, index: usize) {
+        if index < self.views.len() {
+            self.current = index;
+        }
+    }
+
     /// Returns a reference to the specified tab content view.
     pub fn get(&self, index: usize) -> Option<&dyn View> {
         self.views.get(index).map(|(_, view)| view.borrow())
-- 
2.55.0