public inbox for development@lists.ipfire.org
 help / color / mirror / Atom feed
* [PATCH 1/4] zoneconf.cgi: Clean up HTML output
@ 2020-11-17  6:29 Leo-Andres Hofmann
  2020-11-17  6:29 ` [PATCH 2/4] zoneconf.cgi: Make output HTML 5 standard compliant Leo-Andres Hofmann
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Leo-Andres Hofmann @ 2020-11-17  6:29 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 4158 bytes --]

This adds missing brackets, cleans up the indentation and removes unnecessary CSS.

Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
---
 html/cgi-bin/zoneconf.cgi | 46 +++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi
index d99a3e611..067410582 100644
--- a/html/cgi-bin/zoneconf.cgi
+++ b/html/cgi-bin/zoneconf.cgi
@@ -38,10 +38,6 @@ my $css = <<END
 		height: 4em;
 	}
 
-	tr.thin {
-		height: 3em;
-	}
-
 	td.narrow {
 		width: 11em;
 	}
@@ -85,10 +81,6 @@ my $css = <<END
 		border-left-style: none;
 	}
 
-	td.disabled {
-		background-color: #cccccc;
-	}
-
 	td.textcenter {
 		text-align: center;
 	}
@@ -312,8 +304,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 +324,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 +342,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 +353,7 @@ END
 ;
 }
 
-print "</tr>";
+print "\t</tr>\n";
 
 my $slightlygrey = "";
 
@@ -370,7 +362,8 @@ foreach (@nics) {
 	my $nic = $_->[1];
 	my $wlan = $_->[2];
 
-	print "<tr><td class='h narrow textcenter'>$nic<br>$mac</td>";
+	print "\t<tr>\n";
+	print "\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) {
@@ -393,7 +386,12 @@ foreach (@nics) {
 					$checked = "checked";
 				}
 
-				print "<td class='textcenter $slightlygrey'><input type='radio' id='PPPACCESS $mac' name='PPPACCESS' value='$mac' $checked></td>";
+				print <<END
+		<td class="textcenter $slightlygrey">
+			<input type="radio" id="PPPACCESS $mac" name="PPPACCESS" value="$mac" $checked>
+		</td>
+END
+;
 				next; # We're done here
 			}
 		}
@@ -432,19 +430,19 @@ foreach (@nics) {
 		my $vlan_disabled = ($wlan) ? "disabled" : "";
 
 		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 $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>
 END
 ;
 	}
 
-	print "</tr>";
+	print "\t</tr>\n";
 
 	if ($slightlygrey) {
 		$slightlygrey = "";
-- 
2.27.0.windows.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/4] zoneconf.cgi: Make output HTML 5 standard compliant
  2020-11-17  6:29 [PATCH 1/4] zoneconf.cgi: Clean up HTML output Leo-Andres Hofmann
@ 2020-11-17  6:29 ` Leo-Andres Hofmann
  2020-11-17  6:29 ` [PATCH 3/4] zoneconf.cgi: Improve CSS Leo-Andres Hofmann
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Leo-Andres Hofmann @ 2020-11-17  6:29 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 2483 bytes --]

This fixes two minor violations of the HTML standard:
- <a> elements may not contain nested <button> elements:
Replace the button with a simple hyperlink, because it was only used as a link anyway.

- "id" attributes may not contain whitespace:
Remove unneeded attribute, use hyphens instead of spaces.

Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
---
 html/cgi-bin/zoneconf.cgi | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi
index 067410582..2346aa829 100644
--- a/html/cgi-bin/zoneconf.cgi
+++ b/html/cgi-bin/zoneconf.cgi
@@ -99,10 +99,6 @@ my $css = <<END
 	#submit-container.input {
 		margin-left: auto;
 	}
-
-	button {
-		margin-top: 1em;
-	}
 </style>
 END
 ;
@@ -282,7 +278,7 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) {
 	if ($VALIDATE_error) {
 		&Header::openbox('100%', 'left', $Lang::tr{"error"});
 
-		print "$VALIDATE_error<br><a href='/cgi-bin/zoneconf.cgi'><button>$Lang::tr{'ok'}</button></a>";
+		print "$VALIDATE_error<br><br><a href='$ENV{'SCRIPT_NAME'}'>$Lang::tr{'back'}</a>\n";
 
 		&Header::closebox();
 		&Header::closebigbox();
@@ -388,7 +384,7 @@ foreach (@nics) {
 
 				print <<END
 		<td class="textcenter $slightlygrey">
-			<input type="radio" id="PPPACCESS $mac" name="PPPACCESS" value="$mac" $checked>
+			<input type="radio" name="PPPACCESS" value="$mac" $checked>
 		</td>
 END
 ;
@@ -431,12 +427,12 @@ END
 
 		print <<END
 		<td class="textcenter $slightlygrey">
-			<select name="ACCESS $uc $mac" onchange="document.getElementById('TAG $uc $mac').disabled = (this.value === 'VLAN' ? false : true)">
+			<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>
+			<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
 ;
-- 
2.27.0.windows.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 3/4] zoneconf.cgi: Improve CSS
  2020-11-17  6:29 [PATCH 1/4] zoneconf.cgi: Clean up HTML output Leo-Andres Hofmann
  2020-11-17  6:29 ` [PATCH 2/4] zoneconf.cgi: Make output HTML 5 standard compliant Leo-Andres Hofmann
@ 2020-11-17  6:29 ` Leo-Andres Hofmann
  2020-11-17  6:29 ` [PATCH 4/4] zoneconf.cgi: Add NIC selection highlighting Leo-Andres Hofmann
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Leo-Andres Hofmann @ 2020-11-17  6:29 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 4464 bytes --]

- Add an element id so that the styling only affects the zone table
- Alternating row colors are now generated by CSS, remove unneeded Perl code

Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
---
 html/cgi-bin/zoneconf.cgi | 73 ++++++++++++++++++---------------------
 1 file changed, 33 insertions(+), 40 deletions(-)

diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi
index 2346aa829..ef361af95 100644
--- a/html/cgi-bin/zoneconf.cgi
+++ b/html/cgi-bin/zoneconf.cgi
@@ -28,63 +28,64 @@ require "${General::swroot}/header.pl";
 
 my $css = <<END
 <style>
-	table {
+	table#zoneconf {
 		width: 100%;
 		border-collapse: collapse;
 		table-layout: fixed;
 	}
 
-	tr {
+	#zoneconf tr {
 		height: 4em;
 	}
 
-	td.narrow {
-		width: 11em;
+	#zoneconf td {
+		padding: 5px 10px;
+		border: 0.5px solid black;
+		text-align: center;
 	}
 
-	td {
-		padding: 5px;
-		padding-left: 10px;
-		padding-right: 10px;
-		border: 0.5px solid black;
+	/* dark grey header cells */
+	#zoneconf td.heading {
+		background-color: grey;
+		color: white;
+	}	
+	#zoneconf td.heading::first-line {
+		font-weight: bold;
+		line-height: 1.6;
 	}
 
-	td.slightlygrey {
-		background-color: #F0F0F0;
+	/* narrow left column */
+	#zoneconf tr > td:first-child {
+		width: 11em;
 	}
 
-	td.h {
-		background-color: grey;
-		color: white;
-		font-weight: 800;
+	/* alternating row background color */
+	#zoneconf tr:nth-child(2n+3) {
+		background-color: #F0F0F0;
 	}
 
-	td.green {
+	#zoneconf td.green {
 		background-color: $Header::colourgreen;
 	}
 
-	td.red {
+	#zoneconf td.red {
 		background-color: $Header::colourred;
 	}
 
-	td.blue {
+	#zoneconf td.blue {
 		background-color: $Header::colourblue;
 	}
 
-	td.orange {
+	#zoneconf td.orange {
 		background-color: $Header::colourorange;
 	}
 
-	td.topleft {
-		background-color: white;
+	#zoneconf td.topleft {
+		background-color: $Header::pagecolour;
 		border-top-style: none;
 		border-left-style: none;
 	}
 
-	td.textcenter {
-		text-align: center;
-	}
-
 	input.vlanid {
 		width: 4em;
 	}
@@ -299,9 +300,9 @@ if ($cgiparams{"ACTION"} eq $Lang::tr{"save"}) {
 
 print <<END
 <form method='post' enctype='multipart/form-data'>
-	<table>
+	<table id="zoneconf">
 	<tr>
-		<td class="h narrow topleft"></td>
+		<td class="topleft"></td>
 END
 ;
 
@@ -320,7 +321,7 @@ foreach (@zones) {
 		my $red_restricted = ($uc eq "RED" && ! ($red_type eq "STATIC" || $red_type eq "DHCP"));
 
 		if ($red_restricted) {
-			print "\t\t<td class='h textcenter $_'>$uc ($red_type)</td>\n";
+			print "\t\t<td class='heading $_'>$uc ($red_type)</td>\n";
 
 			next; # We're done here
 		}
@@ -338,7 +339,7 @@ foreach (@zones) {
 	}
 
 	print <<END
-		<td class='h textcenter $_'>$uc<br>
+		<td class='heading $_'>$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>
@@ -351,15 +352,13 @@ END
 
 print "\t</tr>\n";
 
-my $slightlygrey = "";
-
 foreach (@nics) {
 	my $mac = $_->[0];
 	my $nic = $_->[1];
 	my $wlan = $_->[2];
 
 	print "\t<tr>\n";
-	print "\t\t<td class='h narrow textcenter'>$nic<br>$mac</td>\n";
+	print "\t\t<td class='heading'>$nic<br>$mac</td>\n";
 
 	# Iterate through all zones and check if the current NIC is assigned to it
 	foreach (@zones) {
@@ -383,7 +382,7 @@ foreach (@nics) {
 				}
 
 				print <<END
-		<td class="textcenter $slightlygrey">
+		<td class="">
 			<input type="radio" name="PPPACCESS" value="$mac" $checked>
 		</td>
 END
@@ -426,7 +425,7 @@ END
 		my $vlan_disabled = ($wlan) ? "disabled" : "";
 
 		print <<END
-		<td class="textcenter $slightlygrey">
+		<td class="">
 			<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>
@@ -439,12 +438,6 @@ END
 	}
 
 	print "\t</tr>\n";
-
-	if ($slightlygrey) {
-		$slightlygrey = "";
-	} else {
-		$slightlygrey = "slightlygrey";
-	}
 }
 
 print <<END
-- 
2.27.0.windows.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 4/4] zoneconf.cgi: Add NIC selection highlighting
  2020-11-17  6:29 [PATCH 1/4] zoneconf.cgi: Clean up HTML output Leo-Andres Hofmann
  2020-11-17  6:29 ` [PATCH 2/4] zoneconf.cgi: Make output HTML 5 standard compliant Leo-Andres Hofmann
  2020-11-17  6:29 ` [PATCH 3/4] zoneconf.cgi: Improve CSS Leo-Andres Hofmann
@ 2020-11-17  6:29 ` Leo-Andres Hofmann
  2020-11-17 10:54 ` [PATCH 1/4] zoneconf.cgi: Clean up HTML output Michael Tremer
  2020-12-01 22:25 ` Thank you for cleaning up the CGIs (was: Re: [PATCH 1/4] zoneconf.cgi: Clean up HTML output) Peter Müller
  4 siblings, 0 replies; 6+ messages in thread
From: Leo-Andres Hofmann @ 2020-11-17  6:29 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 7053 bytes --]

This improves 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 follow changes made by the user.

Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
---
 config/rootfiles/common/web-user-interface |  1 +
 html/cgi-bin/zoneconf.cgi                  | 21 +++++---
 html/html/include/zoneconf.js              | 56 ++++++++++++++++++++++
 3 files changed, 72 insertions(+), 6 deletions(-)
 create mode 100644 html/html/include/zoneconf.js

diff --git a/config/rootfiles/common/web-user-interface b/config/rootfiles/common/web-user-interface
index 44856fcc2..3eac4411a 100644
--- a/config/rootfiles/common/web-user-interface
+++ b/config/rootfiles/common/web-user-interface
@@ -308,6 +308,7 @@ srv/web/ipfire/html/images/wakeup.gif
 srv/web/ipfire/html/images/window-new.png
 srv/web/ipfire/html/include
 srv/web/ipfire/html/include/snortupdateutility.js
+srv/web/ipfire/html/include/zoneconf.js
 srv/web/ipfire/html/index.cgi
 srv/web/ipfire/html/redirect-templates
 srv/web/ipfire/html/redirect-templates/legacy
diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi
index ef361af95..0914ceb78 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#zoneconf {
 		width: 100%;
@@ -101,6 +101,8 @@ my $css = <<END
 		margin-left: auto;
 	}
 </style>
+
+<script src="/include/zoneconf.js"></script>
 END
 ;
 
@@ -151,7 +153,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 ###
@@ -364,6 +366,7 @@ foreach (@nics) {
 	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;
@@ -379,11 +382,12 @@ foreach (@nics) {
 
 				if ($mac eq $ethsettings{"${uc}_MACADDR"}) {
 					$checked = "checked";
+					$highlight = $_;
 				}
 
 				print <<END
-		<td class="">
-			<input type="radio" name="PPPACCESS" value="$mac" $checked>
+		<td class="$highlight">
+			<input type="radio" name="PPPACCESS" value="$mac" data-zone="RED" data-mac="$mac" onchange="highlightAccess(this)" $checked>
 		</td>
 END
 ;
@@ -424,9 +428,14 @@ END
 		$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="">
-			<select name="ACCESS $uc $mac" onchange="document.getElementById('TAG-$uc-$mac').disabled = (this.value === 'VLAN' ? false : true)">
+		<td class="$highlight">
+			<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>
diff --git a/html/html/include/zoneconf.js b/html/html/include/zoneconf.js
new file mode 100644
index 000000000..d76f0ab68
--- /dev/null
+++ b/html/html/include/zoneconf.js
@@ -0,0 +1,56 @@
+/*#############################################################################
+#                                                                             #
+# IPFire.org - A linux based firewall                                         #
+# Copyright (C) 2007-2020  IPFire Team  <info(a)ipfire.org>                     #
+#                                                                             #
+# This program is free software: you can redistribute it and/or modify        #
+# it under the terms of the GNU General Public License as published by        #
+# the Free Software Foundation, either version 3 of the License, or           #
+# (at your option) any later version.                                         #
+#                                                                             #
+# This program is distributed in the hope that it will be useful,             #
+# but WITHOUT ANY WARRANTY; without even the implied warranty of              #
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the               #
+# GNU General Public License for more details.                                #
+#                                                                             #
+# You should have received a copy of the GNU General Public License           #
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.       #
+#                                                                             #
+#############################################################################*/
+
+//zoneconf.cgi dynamic highlighting of interface access selection
+//(call this from "onchange" event of selection elements)
+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'));
+}
-- 
2.27.0.windows.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/4] zoneconf.cgi: Clean up HTML output
  2020-11-17  6:29 [PATCH 1/4] zoneconf.cgi: Clean up HTML output Leo-Andres Hofmann
                   ` (2 preceding siblings ...)
  2020-11-17  6:29 ` [PATCH 4/4] zoneconf.cgi: Add NIC selection highlighting Leo-Andres Hofmann
@ 2020-11-17 10:54 ` Michael Tremer
  2020-12-01 22:25 ` Thank you for cleaning up the CGIs (was: Re: [PATCH 1/4] zoneconf.cgi: Clean up HTML output) Peter Müller
  4 siblings, 0 replies; 6+ messages in thread
From: Michael Tremer @ 2020-11-17 10:54 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 4691 bytes --]

Hello Leo,

This looks like a very clean set of patches. Well done.

I haven’t tested it, but I have seen the previous version in action.

It really looks good.

Best,
-Michael

Reviewed-by: Michael Tremer <michael.tremer(a)ipfire.org>

> On 17 Nov 2020, at 06:29, Leo-Andres Hofmann <hofmann(a)leo-andres.de> wrote:
> 
> This adds missing brackets, cleans up the indentation and removes unnecessary CSS.
> 
> Signed-off-by: Leo-Andres Hofmann <hofmann(a)leo-andres.de>
> ---
> html/cgi-bin/zoneconf.cgi | 46 +++++++++++++++++++--------------------
> 1 file changed, 22 insertions(+), 24 deletions(-)
> 
> diff --git a/html/cgi-bin/zoneconf.cgi b/html/cgi-bin/zoneconf.cgi
> index d99a3e611..067410582 100644
> --- a/html/cgi-bin/zoneconf.cgi
> +++ b/html/cgi-bin/zoneconf.cgi
> @@ -38,10 +38,6 @@ my $css = <<END
> 		height: 4em;
> 	}
> 
> -	tr.thin {
> -		height: 3em;
> -	}
> -
> 	td.narrow {
> 		width: 11em;
> 	}
> @@ -85,10 +81,6 @@ my $css = <<END
> 		border-left-style: none;
> 	}
> 
> -	td.disabled {
> -		background-color: #cccccc;
> -	}
> -
> 	td.textcenter {
> 		text-align: center;
> 	}
> @@ -312,8 +304,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 +324,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 +342,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 +353,7 @@ END
> ;
> }
> 
> -print "</tr>";
> +print "\t</tr>\n";
> 
> my $slightlygrey = "";
> 
> @@ -370,7 +362,8 @@ foreach (@nics) {
> 	my $nic = $_->[1];
> 	my $wlan = $_->[2];
> 
> -	print "<tr><td class='h narrow textcenter'>$nic<br>$mac</td>";
> +	print "\t<tr>\n";
> +	print "\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) {
> @@ -393,7 +386,12 @@ foreach (@nics) {
> 					$checked = "checked";
> 				}
> 
> -				print "<td class='textcenter $slightlygrey'><input type='radio' id='PPPACCESS $mac' name='PPPACCESS' value='$mac' $checked></td>";
> +				print <<END
> +		<td class="textcenter $slightlygrey">
> +			<input type="radio" id="PPPACCESS $mac" name="PPPACCESS" value="$mac" $checked>
> +		</td>
> +END
> +;
> 				next; # We're done here
> 			}
> 		}
> @@ -432,19 +430,19 @@ foreach (@nics) {
> 		my $vlan_disabled = ($wlan) ? "disabled" : "";
> 
> 		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 $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>
> END
> ;
> 	}
> 
> -	print "</tr>";
> +	print "\t</tr>\n";
> 
> 	if ($slightlygrey) {
> 		$slightlygrey = "";
> -- 
> 2.27.0.windows.1
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Thank you for cleaning up the CGIs (was: Re: [PATCH 1/4] zoneconf.cgi: Clean up HTML output)
  2020-11-17  6:29 [PATCH 1/4] zoneconf.cgi: Clean up HTML output Leo-Andres Hofmann
                   ` (3 preceding siblings ...)
  2020-11-17 10:54 ` [PATCH 1/4] zoneconf.cgi: Clean up HTML output Michael Tremer
@ 2020-12-01 22:25 ` Peter Müller
  4 siblings, 0 replies; 6+ messages in thread
From: Peter Müller @ 2020-12-01 22:25 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 646 bytes --]

Hello Leo-Andreas,

sorry for replying late on this; I just wanted to say Hi and thank you 
very much for your efforts on cleaning up this CGI - and perhaps others 
in the future. :-)

Unfortunately, some of those CGIs are not really in a good shape and do 
things in a - let's put it this way - non-optimal fashion. Within the 
past years, there was never enough time to tidy them up one by one.

In case I have understood your messages to that effect that you are 
willing to change this, I absolutely look forward to it and applaud this 
intent.

Please ignore the noise if this is a misunderstanding.

Thanks, and best regards,
Peter Müller

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-12-01 22:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-17  6:29 [PATCH 1/4] zoneconf.cgi: Clean up HTML output Leo-Andres Hofmann
2020-11-17  6:29 ` [PATCH 2/4] zoneconf.cgi: Make output HTML 5 standard compliant Leo-Andres Hofmann
2020-11-17  6:29 ` [PATCH 3/4] zoneconf.cgi: Improve CSS Leo-Andres Hofmann
2020-11-17  6:29 ` [PATCH 4/4] zoneconf.cgi: Add NIC selection highlighting Leo-Andres Hofmann
2020-11-17 10:54 ` [PATCH 1/4] zoneconf.cgi: Clean up HTML output Michael Tremer
2020-12-01 22:25 ` Thank you for cleaning up the CGIs (was: Re: [PATCH 1/4] zoneconf.cgi: Clean up HTML output) Peter Müller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox