From 4771c3a939c45be1cc2814a5d4c2696cb55212f0 Mon Sep 17 00:00:00 2001
From: Mark Wells <mark@freeside.biz>
Date: Thu, 15 Aug 2013 00:08:24 -0700
Subject: [PATCH] fix some UI issues with the report class bulk edit, #24070

---
 httemplate/edit/bulk-part_pkg.html         | 16 ++++-----
 httemplate/edit/process/bulk-part_pkg.html | 23 +++++++-----
 httemplate/elements/checkbox-tristate.html | 57 ++++++++++++++++++++++--------
 3 files changed, 64 insertions(+), 32 deletions(-)

diff --git a/httemplate/edit/bulk-part_pkg.html b/httemplate/edit/bulk-part_pkg.html
index a1c6f0c9b..1773c9a8b 100644
--- a/httemplate/edit/bulk-part_pkg.html
+++ b/httemplate/edit/bulk-part_pkg.html
@@ -22,15 +22,12 @@ The following packages will be changed:<BR>
 % foreach my $num (sort keys %report_class) {
   <TR CLASS="row<%$row % 2%>">
     <TD>
-%   if ( defined $initial_state{$num} ) {
-      <& /elements/checkbox.html,
-            field => 'report_option_'.$num,
-            value => 1,
-            curr_value => $initial_state{$num}
-      &>
-%   } else {
+%   if ( $initial_state{$num} == -1 ) {
 %     # needs to be a tristate so that you can say "don't change it"
       <& /elements/checkbox-tristate.html, field => 'report_option_'.$num &>
+%   } else {
+%# for visual consistency
+      <INPUT TYPE="checkbox" CLASS="partial" NAME="report_option_<%$num%>" VALUE="1" <% $initial_state{$num} ? 'CHECKED':'' %>><LABEL />
 %   }
     </TD>
     <TD><% $report_class{$num}->name %></TD>
@@ -64,11 +61,12 @@ foreach my $num (keys %report_class) {
     }
   }
   if ( $yes and $no ) {
-    $initial_state{$num} = undef;
+    $initial_state{$num} = -1;
   } elsif ( $yes ) {
     $initial_state{$num} = 1;
   } elsif ( $no ) {
-    $initial_state{$num} = 0;
+    $initial_state{$num} = '';
   } # else, uh, you didn't provide any pkgparts
 }
+warn Dumper \%initial_state;
 </%init>
diff --git a/httemplate/edit/process/bulk-part_pkg.html b/httemplate/edit/process/bulk-part_pkg.html
index 4775a9334..f546701e8 100644
--- a/httemplate/edit/process/bulk-part_pkg.html
+++ b/httemplate/edit/process/bulk-part_pkg.html
@@ -10,21 +10,26 @@ die "access denied" unless $FS::CurrentUser::CurrentUser->access_right('Bulk edi
 my @pkgparts = $cgi->param('pkgpart')
   or die "no package definitions selected";
 
-my %changes;
+my %delete = map { 'report_option_'.($_->num) => 1 }
+  qsearch('part_pkg_report_option', {});
+my %insert;
+
 foreach my $param (grep { /^report_option_\d+$/ } $cgi->param) {
-  if ( length($cgi->param($param)) ) {
-    if ( $cgi->param($param) == 1 ) {
-      $changes{$param} = 1;
-    } else {
-      $changes{$param} = '';
-    }
-  }
+  if ( $cgi->param($param) == 1 ) {
+    $insert{$param} = 1;
+    delete $delete{$param};
+  } elsif ( $cgi->param($param) == -1 ) {
+    # leave it alone
+    delete $delete{$param};
+  } # else it's empty, so leave it on the delete list
 }
 
+
 my $error;
 foreach my $pkgpart (@pkgparts) {
   my $part_pkg = FS::part_pkg->by_key($pkgpart);
-  my %options = ( $part_pkg->options, %changes );
+  my %options = ( $part_pkg->options, %insert );
+  delete $options{$_} foreach keys(%delete);
   $error ||= $part_pkg->replace( options => \%options );
 }
 </%init>
diff --git a/httemplate/elements/checkbox-tristate.html b/httemplate/elements/checkbox-tristate.html
index 4c26ed74e..90966a509 100644
--- a/httemplate/elements/checkbox-tristate.html
+++ b/httemplate/elements/checkbox-tristate.html
@@ -2,29 +2,54 @@
 A tristate checkbox (with three values: true, false, and null).
 Internally, this creates a checkbox, coupled via javascript to a hidden
 field that actually contains the value.  For now, the only values these
-can have are 1, 0, and empty.  Clicking the checkbox cycles between them.
+can have are 1, -1, and empty.  Clicking the checkbox cycles between them.
+
+For compatibility with regular checkboxes, empty is the false state and 
+-1 is the indeterminate state.
+
+Displaying these is a problem.  "indeterminate" is a standard HTML5 attribute
+but some browsers display it in unhelpful ways (e.g. Firefox slightly grays
+the checkbox, approximately #dddddd), and checkboxes ignore nearly all CSS 
+styling.
 </%doc>
 <%shared>
 my $init = 0;
 </%shared>
 % if ( !$init ) {
 %   $init = 1;
+<STYLE>
+input.partial {
+  position: absolute;
+  opacity: 0;
+  z-index: 1;
+}
+input.partial + label::before {
+  position: relative;
+  content: "\2610";
+}
+input.partial:checked + label::before {
+  content: "\2611";
+}
+input.partial:indeterminate + label::before {
+  content: "\2612";
+}
+</STYLE>
 <SCRIPT TYPE="text/javascript">
 function tristate_onclick() {
   var checkbox = this;
   var input = checkbox.input;
-  if ( input.value == "" ) {
-    input.value = "0";
-    checkbox.checked = false;
-    checkbox.indeterminate = false;
-  } else if ( input.value == "0" ) {
+  if ( input.value == "" ) { // false -> true
     input.value = "1";
     checkbox.checked = true;
     checkbox.indeterminate = false;
-  } else if ( input.value == "1" ) {
+  } else if ( input.value == "1" ) { // true -> indeterminate
+    input.value = "-1";
+    checkbox.checked = false;
+    checkbox.indeterminate = true;
+  } else if ( input.value == "-1" ) { // indeterminate -> false
     input.value = "";
-    checkbox.checked = true;
-    checkbox.indeterminate = true
+    checkbox.checked = false;
+    checkbox.indeterminate = false;
   }
 }
 
@@ -47,10 +72,10 @@ window.onload = function() { // don't do this until all of the checkboxes exist
     // set event handler
     tristate_boxes[i].onclick = tristate_onclick;
     // set initial value
-    if ( tristates[i].value == "" ) {
+    if ( tristates[i].value == "-1" ) {
       tristate_boxes[i].indeterminate = true
     }
-    if ( tristates[i].value != "0" ) {
+    if ( tristates[i].value == "1" ) {
       tristate_boxes[i].checked = true;
     }
   }
@@ -62,6 +87,7 @@ window.onload = function() { // don't do this until all of the checkboxes exist
                      VALUE="<% $curr_value %>"
                      CLASS="tristate">
 <INPUT TYPE="checkbox" ID="checkbox_<%$opt{field}%>" CLASS="partial">
+<LABEL />
 <%init>
 
 my %opt = @_;
@@ -72,7 +98,10 @@ my %opt = @_;
 #                 : '';
 
 $opt{'id'} ||= 'hidden_'.$opt{'field'};
-my $curr_value = $opt{curr_value};
-$curr_value = undef
-  unless $curr_value eq '0' or $curr_value eq '1';
+my $curr_value = '-1';
+if (exists $opt{curr_value}) {
+  $curr_value = $opt{curr_value};
+  $curr_value = '' unless $curr_value eq '-1' or $curr_value eq '1';
+}
+
 </%init>
-- 
2.11.0