Skip to content

Commit b035db2

Browse files
gsanchiettigsanchietti
committed
feat(setup): improve UX
Changes: - confirm exit when there are unsaved network changes - validate IP/CIDR input immediately on entry, re-prompt on error - split network menu into interface selection and configuration pages - move apply changes item under network menu - improve labels - rename Cancel buttons to Back in sub-menus and Exit in main menu Assisted-by: Copilot/Various models
1 parent 0185ec2 commit b035db2

1 file changed

Lines changed: 98 additions & 52 deletions

File tree

  • packages/ns-api/files

‎packages/ns-api/files/setup‎

Lines changed: 98 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -297,13 +297,13 @@ load_zone_interfaces() {
297297

298298
device_record() {
299299
device_name="$1"
300-
awk -F '\t' -v device="$device_name" '$1 == device { print; exit }' "$DEVICE_DB"
300+
awk -F '\t' -v device="$device_name" '$1 == device { print; exit }' "$DEVICE_DB" 2>/dev/null
301301
}
302302

303303
device_field() {
304304
device_name="$1"
305305
field_number="$2"
306-
awk -F '\t' -v device="$device_name" -v field="$field_number" '$1 == device { print $field; exit }' "$DEVICE_DB"
306+
awk -F '\t' -v device="$device_name" -v field="$field_number" '$1 == device { print $field; exit }' "$DEVICE_DB" 2>/dev/null
307307
}
308308

309309
device_kind() {
@@ -603,7 +603,7 @@ initialize_state() {
603603
}
604604

605605
pick_keymap() {
606-
run_dialog --title "Keymap" --radiolist "Select keymap" 12 60 2 \
606+
run_dialog --title "Keymap" --cancel-button "Cancel" --radiolist "Select keyboard layout (keymap)" 12 60 2 \
607607
it "Italian" "$( [ "$KEYMAP" = "it" ] && printf ON || printf OFF )" \
608608
us "US (default)" "$( [ "$KEYMAP" = "us" ] && printf ON || printf OFF )" || return 0
609609

@@ -665,19 +665,19 @@ prompt_value() {
665665
}
666666

667667
pick_lan_iface() {
668-
choice=$(pick_interface "Lan Interface" "Select the LAN interface" "$LAN_IFACE" "$LAN_INTERFACES") || return 0
668+
choice=$(pick_interface "LAN Interface" "Select the LAN interface" "$LAN_IFACE" "$LAN_INTERFACES") || return 0
669669
load_iface_state lan "$choice"
670670
save_state
671671
}
672672

673673
pick_lan_device() {
674-
choice=$(pick_device "Lan Device" "Select the LAN network device" "$LAN_DEVICE") || return 0
674+
choice=$(pick_device "LAN Device" "Select the LAN network device" "$LAN_DEVICE") || return 0
675675
LAN_DEVICE="$choice"
676676
save_state
677677
}
678678

679679
pick_lan_proto() {
680-
run_dialog --title "Lan Protocol" --radiolist "Select the LAN protocol" 12 60 2 \
680+
run_dialog --title "LAN Protocol" --radiolist "Select the LAN protocol" 12 60 2 \
681681
dhcp "Automatic configuration" "$( [ "$LAN_PROTO" = "dhcp" ] && printf ON || printf OFF )" \
682682
static "Static IPv4 configuration" "$( [ "$LAN_PROTO" = "static" ] && printf ON || printf OFF )" \
683683
|| return 0
@@ -694,28 +694,34 @@ pick_lan_proto() {
694694

695695
pick_lan_ip() {
696696
[ "$LAN_PROTO" = "static" ] || {
697-
whiptail --title "Lan IPv4" --msgbox "LAN IPv4 address is only required for static LAN." 10 70
697+
whiptail --title "LAN IPv4" --msgbox "LAN IPv4 address is only required for static LAN." 10 70
698698
return 0
699699
}
700-
value=$(prompt_value "Lan IPv4" "Enter LAN IPv4 address in CIDR notation" "$LAN_IP") || return 0
701-
LAN_IP="$value"
702-
save_state
700+
while true; do
701+
value=$(prompt_value "LAN IPv4" "Enter LAN IPv4 address in CIDR notation (e.g. 192.168.1.1/24)" "$LAN_IP") || return 0
702+
if validate_cidr "$value"; then
703+
LAN_IP="$value"
704+
save_state
705+
return 0
706+
fi
707+
whiptail --title "LAN IPv4" --msgbox "Invalid address: '$value'\nExpected format: x.x.x.x/prefix (e.g. 192.168.1.1/24)" 10 70
708+
done
703709
}
704710

705711
pick_wan_iface() {
706-
choice=$(pick_interface "Wan Interface" "Select the WAN interface" "$WAN_IFACE" "$WAN_INTERFACES") || return 0
712+
choice=$(pick_interface "WAN Interface" "Select the WAN interface" "$WAN_IFACE" "$WAN_INTERFACES") || return 0
707713
load_iface_state wan "$choice"
708714
save_state
709715
}
710716

711717
pick_wan_device() {
712-
choice=$(pick_device "Wan Device" "Select the WAN network device" "$WAN_DEVICE") || return 0
718+
choice=$(pick_device "WAN Device" "Select the WAN network device" "$WAN_DEVICE") || return 0
713719
WAN_DEVICE="$choice"
714720
save_state
715721
}
716722

717723
pick_wan_proto() {
718-
run_dialog --title "Wan Protocol" --radiolist "Select the WAN protocol" 12 60 2 \
724+
run_dialog --title "WAN Protocol" --radiolist "Select the WAN protocol" 12 60 2 \
719725
dhcp "Automatic configuration" "$( [ "$WAN_PROTO" = "dhcp" ] && printf ON || printf OFF )" \
720726
static "Static IPv4 configuration" "$( [ "$WAN_PROTO" = "static" ] && printf ON || printf OFF )" \
721727
|| return 0
@@ -735,58 +741,95 @@ pick_wan_proto() {
735741

736742
pick_wan_ip() {
737743
[ "$WAN_PROTO" = "static" ] || {
738-
whiptail --title "Wan IPv4" --msgbox "WAN IPv4 address is only required for static WAN." 10 70
744+
whiptail --title "WAN IPv4" --msgbox "WAN IPv4 address is only required for static WAN." 10 70
739745
return 0
740746
}
741-
value=$(prompt_value "Wan IPv4" "Enter WAN IPv4 address in CIDR notation" "$WAN_IP") || return 0
742-
WAN_IP="$value"
743-
save_state
747+
while true; do
748+
value=$(prompt_value "WAN IPv4" "Enter WAN IPv4 address in CIDR notation (e.g. 1.2.3.4/24)" "$WAN_IP") || return 0
749+
if validate_cidr "$value"; then
750+
WAN_IP="$value"
751+
save_state
752+
return 0
753+
fi
754+
whiptail --title "WAN IPv4" --msgbox "Invalid address: '$value'\nExpected format: x.x.x.x/prefix (e.g. 1.2.3.4/24)" 10 70
755+
done
744756
}
745757

746758
pick_wan_gw() {
747759
[ "$WAN_PROTO" = "static" ] || {
748-
whiptail --title "Wan Gateway" --msgbox "WAN gateway is only required for static WAN." 10 70
760+
whiptail --title "WAN Gateway" --msgbox "WAN gateway is only required for static WAN." 10 70
749761
return 0
750762
}
751-
value=$(prompt_value "Wan Gateway" "Enter WAN IPv4 gateway" "$WAN_GW") || return 0
752-
WAN_GW="$value"
753-
save_state
763+
while true; do
764+
value=$(prompt_value "WAN Gateway" "Enter WAN IPv4 gateway (e.g. 1.2.3.1)" "$WAN_GW") || return 0
765+
if validate_ipv4 "$value"; then
766+
WAN_GW="$value"
767+
save_state
768+
return 0
769+
fi
770+
whiptail --title "WAN Gateway" --msgbox "Invalid address: '$value'\nExpected a valid IPv4 address (e.g. 1.2.3.1)" 10 70
771+
done
754772
}
755773

756774
network_menu() {
757775
while true; do
758-
set -- --title "Network" --menu "Choose a network setting" 18 70 7 \
759-
"Lan Interface" "${LAN_IFACE:-unset}" \
760-
"Lan" "${LAN_DEVICE:-unset}" \
761-
"Lan Protocol" "${LAN_PROTO:-unset}" \
762-
"Wan Interface" "${WAN_IFACE:-unset}" \
763-
"Wan" "${WAN_DEVICE:-unset}" \
764-
"Wan Protocol" "${WAN_PROTO:-unset}"
776+
run_dialog --title "Network" --cancel-button "Back" --menu "Select the interface to configure" 14 70 3 \
777+
LAN "Configure LAN, local network" \
778+
WAN "Configure WAN, external network" \
779+
apply "Apply network changes" || return 0
780+
781+
case "$WHIPTAIL_RESULT" in
782+
LAN) lan_menu ;;
783+
WAN) wan_menu ;;
784+
apply) apply_changes ;;
785+
esac
786+
done
787+
}
788+
789+
lan_menu() {
790+
while true; do
791+
set -- --title "LAN" --cancel-button "Back" --menu "Configure the LAN interface" 16 70 5 \
792+
"Interface" "${LAN_IFACE:-unset}" \
793+
"Device" "${LAN_DEVICE:-unset}" \
794+
"Protocol" "${LAN_PROTO:-unset}"
765795
if [ "$LAN_PROTO" = "static" ]; then
766796
set -- "$@" \
767-
"Lan IPv4" "${LAN_IP:-unset}"
797+
"IPv4" "${LAN_IP:-unset}"
768798
fi
799+
800+
run_dialog "$@" || return 0
801+
selection="$WHIPTAIL_RESULT"
802+
803+
case "$selection" in
804+
"Interface") pick_lan_iface ;;
805+
"Device") pick_lan_device ;;
806+
"Protocol") pick_lan_proto ;;
807+
"IPv4") pick_lan_ip ;;
808+
esac
809+
done
810+
}
811+
812+
wan_menu() {
813+
while true; do
814+
set -- --title "WAN" --cancel-button "Back" --menu "Configure the WAN interface" 16 70 6 \
815+
"Interface" "${WAN_IFACE:-unset}" \
816+
"Device" "${WAN_DEVICE:-unset}" \
817+
"Protocol" "${WAN_PROTO:-unset}"
769818
if [ "$WAN_PROTO" = "static" ]; then
770819
set -- "$@" \
771-
"Wan IPv4" "${WAN_IP:-unset}" \
772-
"Wan Gateway" "${WAN_GW:-unset}"
820+
"IPv4" "${WAN_IP:-unset}" \
821+
"Gateway" "${WAN_GW:-unset}"
773822
fi
774-
set -- "$@" "Back" "Return to main menu"
775823

776824
run_dialog "$@" || return 0
777825
selection="$WHIPTAIL_RESULT"
778826

779827
case "$selection" in
780-
"Lan Interface") pick_lan_iface ;;
781-
"Lan") pick_lan_device ;;
782-
"Lan Protocol") pick_lan_proto ;;
783-
"Lan IPv4") pick_lan_ip ;;
784-
"Wan Interface") pick_wan_iface ;;
785-
"Wan") pick_wan_device ;;
786-
"Wan Protocol") pick_wan_proto ;;
787-
"Wan IPv4") pick_wan_ip ;;
788-
"Wan Gateway") pick_wan_gw ;;
789-
"Back") return 0 ;;
828+
"Interface") pick_wan_iface ;;
829+
"Device") pick_wan_device ;;
830+
"Protocol") pick_wan_proto ;;
831+
"IPv4") pick_wan_ip ;;
832+
"Gateway") pick_wan_gw ;;
790833
esac
791834
done
792835
}
@@ -1038,7 +1081,7 @@ wan_has_changes() {
10381081

10391082
apply_changes() {
10401083
if ! network_has_changes; then
1041-
whiptail --title "Setup" --msgbox "Keymap changes are applied immediately. No network changes to apply." 10 70
1084+
whiptail --title "Setup" --msgbox "No network changes to apply." 10 70
10421085
return 0
10431086
fi
10441087

@@ -1055,8 +1098,8 @@ apply_changes() {
10551098
else
10561099
lan_ip_summary='-'
10571100
fi
1058-
summary=$(printf 'Keymap: %s\nLan Device: %s\nLan Protocol: %s\nLan IPv4/CIDR: %s\nWan Device: %s\nWan Protocol: %s\nWan IPv4/CIDR: %s\nWan Gateway: %s' \
1059-
"$KEYMAP" "$LAN_DEVICE" "$LAN_PROTO" "$lan_ip_summary" "$WAN_DEVICE" "$WAN_PROTO" "$(display_value "$WAN_IP")" "$(display_value "$WAN_GW")")
1101+
summary=$(printf 'LAN Device: %s\nLAN Protocol: %s\nLAN IPv4/CIDR: %s\nWAN Device: %s\nWAN Protocol: %s\nWAN IPv4/CIDR: %s\nWAN Gateway: %s' \
1102+
"$LAN_DEVICE" "$LAN_PROTO" "$lan_ip_summary" "$WAN_DEVICE" "$WAN_PROTO" "$(display_value "$WAN_IP")" "$(display_value "$WAN_GW")")
10601103
whiptail --title "Confirm Changes" --yesno "$summary\n\nApply these changes?" 18 78 || return 0
10611104

10621105
if lan_has_changes; then
@@ -1087,20 +1130,23 @@ ensure_root() {
10871130
[ "$(id -u)" -eq 0 ] || die "This command must be run as root."
10881131
}
10891132

1133+
confirm_exit() {
1134+
network_has_changes || return 0
1135+
whiptail --title "Exit" --yesno "You have unsaved network changes.\nExit without applying?" 10 70
1136+
}
1137+
10901138
main_menu() {
10911139
while true; do
1092-
run_dialog --title "Setup" --menu "Choose an action" 16 70 4 \
1093-
network "Network" \
1094-
keymap "Keymap: ${KEYMAP}" \
1095-
apply "Apply Changes" \
1096-
exit "Exit Without Applying" || break
1140+
run_dialog --title "Setup" --cancel-button "Exit" --menu "Choose an action" 14 70 3 \
1141+
network "Configure network" \
1142+
keymap "Keyboard layout: ${KEYMAP}" \
1143+
exit "Exit without applying" || { confirm_exit && break; continue; }
10971144
selection="$WHIPTAIL_RESULT"
10981145

10991146
case "$selection" in
11001147
keymap) pick_keymap ;;
11011148
network) network_menu ;;
1102-
apply) apply_changes ;;
1103-
exit) break ;;
1149+
exit) confirm_exit && break ;;
11041150
esac
11051151
done
11061152

0 commit comments

Comments
 (0)