Re: [PATCH] zoneconf.cgi: Improve the usability of the zone configuration by marking assigned NICs in the zone color. The highlighting is initially applied to the static HTML output, and JavaScript is used to dynamically follow changes made by the user.

Hello Regardless of the advice and improvement requests of others, I have applied the patch to my IPFire. So far everything works as expected and I could not find any errors or misbehaviour.

Greetings Daniel

Am 10.11.2020 um 13:37 schrieb Leo-Andres Hofmann:

Discussion: https://lists.ipfire.org/pipermail/development/2020-October/008567.html

Signed-off-by: Leo-Andres Hofmann hofmann@leo-andres.de

html/cgi-bin/zoneconf.cgi | 89 +++++++++++++++++++++++++++++---------- 1 file changed, 67 insertions(+), 22 deletions(-)

diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi index d99a3e611..2501901c4 100644 --- a/html/cgi-bin/zoneconf.cgi +++ b/html/cgi-bin/zoneconf.cgi @@ -26,7 +26,7 @@ require '/var/ipfire/general-functions.pl'; require "${General::swroot}/lang.pl"; require "${General::swroot}/header.pl";

-my $css = <<END +my $extraHead = <<END

<style> table { width: 100%; @@ -64,19 +64,19 @@ my $css = <<END } td.green { - background-color: $Header::colourgreen; + background-color: $Header::colourgreen !important; } td.red { - background-color: $Header::colourred; + background-color: $Header::colourred !important; } td.blue { - background-color: $Header::colourblue; + background-color: $Header::colourblue !important; } td.orange { - background-color: $Header::colourorange; + background-color: $Header::colourorange !important; } td.topleft { @@ -112,6 +112,44 @@ my $css = <<END margin-top: 1em; } </style>

+<script type="text/javascript">

• //highlight interface access selection, call this from "onchange" event of selection element
• function highlightAccess(selectObj) {

• if(!(selectObj && ('zone' in selectObj.dataset) && ('mac' in selectObj.dataset))) {
  

• 	return; //required parameters are missing
  

• }
  

• var zoneColor = selectObj.dataset.zone.trim().toLowerCase(); //zone color (red/green/blue/orange) CSS class
  

• function colorParentCell(obj, color, enabled = true) { //find nearest parent table cell of "obj" and set its CSS color class
  

• 	do {
  

• 		obj = obj.parentElement;
  

• 	} while(obj && (obj.nodeName.toUpperCase() !== 'TD'));
  

• 	if(obj && (['green', 'red', 'orange', 'blue'].includes(color))) {
  

• 		obj.classList.toggle(color, enabled);
  

• 	}
  

• }
  

• //handle other associated input fields
  

• if(selectObj.type.toUpperCase() === 'RADIO') { //this is a radio button group: clear all highlights
  

• 	let radios = document.getElementsByName(selectObj.name);
  

• 	radios.forEach(function(button) {
  

• 		if(button.nodeName.toUpperCase() === 'INPUT') { //make sure the found element is a button
  

• 			colorParentCell(button, zoneColor, false); //remove css
  

• 		}
  

• 	});
  

• } else { //this is a dropdown menu: enable/disable additional VLAN tag input box
  

• 	let tagInput = document.getElementById('TAG ' + selectObj.dataset.zone + ' ' + selectObj.dataset.mac); //tag input element selector
  

• 	if(tagInput) {
  

• 		tagInput.disabled = (selectObj.value !== 'VLAN'); //enable tag input if VLAN mode selected
  

• 	}
  

• }
  

• //if interface is assigned, highlight table cell in zone color
  

• colorParentCell(selectObj, zoneColor, (selectObj.value !== 'NONE'));
  

• }

+</script> END ;

@@ -162,7 +200,7 @@ foreach (@nics) { } }

-&Header::openpage($Lang::tr{"zoneconf title"}, 1, $css); +&Header::openpage($Lang::tr{"zoneconf title"}, 1, $extraHead); &Header::openbigbox('100%', 'center');

### Evaluate POST parameters ### @@ -312,8 +350,8 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) { print <<END

<form method='post' enctype='multipart/form-data'> <table> - <tr> - <td class="h narrow topleft" /td> + <tr> + <td class="h narrow topleft"></td> END ;

@@ -332,7 +370,7 @@ foreach (@zones) { my $red_restricted = ($uc eq "RED" && ! ($red_type eq "STATIC" || $red_type eq "DHCP"));

if ($red_restricted) {

• 	print "<td class='h textcenter $_'>$uc ($red_type)</td>";
  

• 	print "\t\t<td class='h textcenter $_'>$uc ($red_type)</td>\n";
  
  next; # We're done here
  

  }

@@ -350,7 +388,7 @@ foreach (@zones) { }

print <<END

• <td class='h textcenter $_'>$uc</br>
  

• <td class='h textcenter $_'>$uc<br>
  <select name="MODE $uc">
  	<option value="DEFAULT" $mode_selected{"DEFAULT"}>$Lang::tr{"zoneconf nicmode default"}</option>
  	<option value="BRIDGE" $mode_selected{"BRIDGE"}>$Lang::tr{"zoneconf nicmode bridge"}</option>
  

@@ -361,7 +399,7 @@ END ; }

-print "</tr>"; +print "\t</tr>\n";

my $slightlygrey = "";

@@ -370,12 +408,13 @@ foreach (@nics) { my $nic = $_->[1]; my $wlan = $_->[2];

• print "<tr><td class='h narrow textcenter'>$nic<br>$mac</td>";

• print "\t<tr>\n\t\t<td class='h narrow textcenter'>$nic<br>$mac</td>\n";

  # Iterate through all zones and check if the current NIC is assigned to it foreach (@zones) { my $uc = uc $_; my $dev_name = $ethsettings{"${uc}_DEV"};

• my $highlight = "";
  
  

  if ($dev_name eq "") { # Again, skip the zone if it is not activated next;

@@ -391,9 +430,10 @@ foreach (@nics) {

		if ($mac eq $ethsettings{"${uc}_MACADDR"}) {
			$checked = "checked";

• 			$highlight = $_;
  	}
  
  

• 		print "<td class='textcenter $slightlygrey'><input type='radio' id='PPPACCESS $mac' name='PPPACCESS' value='$mac' $checked></td>";
  

• 		print "\t\t<td class='textcenter $highlight $slightlygrey'><input type='radio' id='PPPACCESS $mac' name='PPPACCESS' value='$mac' data-zone='RED' data-mac='$mac' onchange='highlightAccess(this)' $checked></td>\n";
  	next; # We're done here
  }
  

  }

@@ -430,21 +470,26 @@ foreach (@nics) {

$access_selected{"NONE"} = ($access_selected{"NATIVE"} eq "") && ($access_selected{"VLAN"} eq "") ? "selected" : "";
my $vlan_disabled = ($wlan) ? "disabled" : "";

• # If the interface is assigned, hightlight table cell
  

• if ($access_selected{"NONE"} eq "") {
  

• 	$highlight = $_;
  

• }
  
  

  print <<END

• 	<td class="textcenter $slightlygrey">
  

• 		<select name="ACCESS $uc $mac" onchange="document.getElementById('TAG $uc $mac').disabled = (this.value === 'VLAN' ? false : true)">
  

• 			<option value="NONE" $access_selected{"NONE"}>- $Lang::tr{"zoneconf access none"} -</option>
  

• 			<option value="NATIVE" $access_selected{"NATIVE"}>$Lang::tr{"zoneconf access native"}</option>
  

• 			<option value="VLAN" $access_selected{"VLAN"} $vlan_disabled>$Lang::tr{"zoneconf access vlan"}</option>
  

• 		</select>
  

• 		<input type="number" class="vlanid" id="TAG $uc $mac" name="TAG $uc $mac" min="1" max="4095" value="$zone_vlan_id" $field_disabled>
  

• 	</td>
  

• <td class="textcenter $highlight $slightlygrey">
  

• 	<select name="ACCESS $uc $mac" data-zone="$uc" data-mac="$mac" onchange="highlightAccess(this)">
  

• 		<option value="NONE" $access_selected{"NONE"}>- $Lang::tr{"zoneconf access none"} -</option>
  

• 		<option value="NATIVE" $access_selected{"NATIVE"}>$Lang::tr{"zoneconf access native"}</option>
  

• 		<option value="VLAN" $access_selected{"VLAN"} $vlan_disabled>$Lang::tr{"zoneconf access vlan"}</option>
  

• 	</select>
  

• 	<input type="number" class="vlanid" id="TAG $uc $mac" name="TAG $uc $mac" min="1" max="4095" value="$zone_vlan_id" $field_disabled>
  

• </td>
  

  END ; }

• print "</tr>";

• print "\t</tr>\n";

  if ($slightlygrey) { $slightlygrey = "";