Bug 22168: Improve styling of new chart settings for reports
authorOwen Leonard <oleonard@myacpl.org>
Fri, 18 Jan 2019 17:10:40 +0000 (17:10 +0000)
committerMartin Renvoize <martin.renvoize@ptfs-europe.com>
Fri, 1 Feb 2019 15:28:14 +0000 (15:28 +0000)
This patch updates a number of aspects of the reports chart creation
interface: Chart creation form is now shown in a modal window, and a
separate link has been added to show or hide the chart itself.

Also changed: Minor markup and JavaScript cleanup.

To test, apply the patch and go to Reports -> Saved reports.

- Run a report which returns more than one column.
- On the report results page, click the "Create chart" button. The chart
  settings form should appear in a modal window.
- Confirm that the form controls work correctly: Each label should give
  focus to the correct input. Changing the chart type should show or
  hide the appropriate form fields.
- Click the "Draw" button. The modal should disappear and the chart
  should be shown.
- Above the chart should be a "Hide chart" link which works to hide (and
  then show) the chart.

Signed-off-by: Pierre-Marc Thibault <pierre-marc.thibault@inLibro.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
(cherry picked from commit 8e1efe8f10fd5b575b2ceea3b33d1ed9dd91bd96)
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

koha-tmpl/intranet-tmpl/prog/css/reports.css
koha-tmpl/intranet-tmpl/prog/en/includes/chart.inc
koha-tmpl/intranet-tmpl/prog/en/includes/reports-toolbar.inc
koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt

index 4ccee11..2152cc8 100644 (file)
@@ -72,4 +72,40 @@ del {
 
 .clear_filter i {
     color: #C00;
+}
+
+fieldset.rows {
+    background-color: transparent;
+    border: 0;
+    font-size: 100%;
+    margin: 0;
+    padding: 0;
+}
+
+fieldset.rows label,
+fieldset.rows span.label {
+    width: 12em;
+}
+
+fieldset.rows legend {
+    margin-left: 0;
+}
+
+fieldset.rows .column_config_row {
+    display: flex;
+    flex-wrap: wrap;
+    justify-content: flex-start;
+}
+
+fieldset.rows .chart-column-conf {
+    padding: .7em;
+    width: auto;
+}
+
+fieldset.rows .chart-column-conf label {
+    display: block;
+    float: none;
+    margin: .4em 0;
+    text-align: left;
+    width: auto;
 }
\ No newline at end of file
index 03a28f0..cb1f6d3 100644 (file)
-<div id="makechart" style="display:none;">
-    [% supposed_x = header_row.shift.cell %]
-
-    <fieldset>
-        <legend>Draw a chart</legend>
-        <ol>
-            <li>
-                <label for="chart-type">Chart type</label>
-                <select name="chart-type" id="chart-type">
-                    <option value="pie">Pie</option>
-                    <option value="bar">Bar</option>
-                    <option value="line">Line</option>
-                </select>
-
-                <div id="chart-column-horizontal">
-                    <label for="horizontal">Horizontal bar:</label>
-                    <input id="horizontal" name="column-horizontal" type="checkbox">
-                </div>
-            </li>
-
-            <li>
-                <label for="x_element">x column:</label>
-                <select id="x_element" name="x">
-                    <option value="[% supposed_x | html %]" selected>[% supposed_x | html %]</option>
-                    [% FOREACH header IN header_row %]
-                        <option value="[% header.cell | html %]">[% header.cell | html %]</option>
-                    [% END %]
-                </select>
-            </li>
-
-            <div>
-                <label for="include-all">Include all rows (ignore pagination)</label>
-                <input id="include-all" name="chart-include-all" type="checkbox">
+<div class="modal" id="chartModal" tabindex="-1" role="dialog" aria-labelledby="chartModalLabel">
+    <div class="modal-dialog modal-wide" role="document">
+        <div class="modal-content">
+            <div class="modal-header">
+                <button type="button" class="closebtn" data-dismiss="modal" aria-label="Close"><span aria-hidden="true">&times;</span></button>
+                <h4 class="modal-title" id="chartModalLabel">Chart settings</h4>
             </div>
+            <div class="modal-body" id="makechart">
+                [% supposed_x = header_row.shift.cell %]
+                <fieldset class="rows">
+                    <ol>
+                        <li>
+                            <label for="chart-type">Chart type: </label>
+                            <select name="chart-type" id="chart-type">
+                                <option value="pie">Pie</option>
+                                <option value="bar">Bar</option>
+                                <option value="line">Line</option>
+                            </select>
+                        </li>
 
+                        <li id="chart-column-horizontal">
+                            <label for="horizontal">Horizontal bar:</label>
+                            <input id="horizontal" name="column-horizontal" type="checkbox" />
+                        </li>
 
-            <label for="exclude-last">Exclude last line (Rollup)</label>
-            <input id="exclude-last" name="chart-exclude-last" type="checkbox">
-
-            [% column = 1 %]
-            <li>
-                [% FOREACH header IN header_row %]
-                    <fieldset class="chart-column-conf" id="column_[% column | html %]" style="display: inline !important;">
-                        <legend>
-                            Column [% column | html %]
-                            <a class="chart-column-delete" href="#" data-column="[% column | html %]">
-                                <img src="[% interface | html %]/[% theme | html %]/img/x.png" alt="Delete" />
-                            </a>
-                        </legend>
-                        <div>
-                            <label for="y_[% column | html %]" >y:</label>
-                            <select id="y_[% column | html %]" name="y">
+                        <li>
+                            <label for="x_element">x column:</label>
+                            <select id="x_element" name="x">
                                 <option value="[% supposed_x | html %]" selected>[% supposed_x | html %]</option>
-                                [% FOREACH h IN header_row %]
-                                    [% IF header.cell == h.cell %]
-                                        <option value="[% h.cell | html %]" selected>[% h.cell | html %]</option>
-                                    [% ELSE %]
-                                        <option value="[% h.cell | html %]">[% h.cell | html %]</option>
-                                    [% END %]
-                            [% END %]
-                            </select>
-                        </div>
-
-                        <div class="chart-column-group">
-                            [% i = 1 %]
-                            <label for="group_[% column | html %]">Group:</label>
-                            <select id="group_[% column | html %]" name="group">
-                                [% FOREACH h IN header_row %]
-                                    [% IF i == column %]
-                                        <option value="[% i | html %]" selected>[% i | html %]</option>
-                                    [% ELSE %]
-                                        <option value="[% i | html %]">[% i | html %]</option>
-                                    [% END %]
-                                    [% i = i + 1 %]
+                                [% FOREACH header IN header_row %]
+                                    <option value="[% header.cell | html %]">[% header.cell | html %]</option>
                                 [% END %]
                             </select>
-                        </div>
+                        </li>
+
+                        <li>
+                            <label for="include-all">Include all rows (ignore pagination):</label>
+                            <input id="include-all" name="chart-include-all" type="checkbox" />
+                        </li>
 
-                        <div class="chart-column-line">
-                            <label for="line_[% column | html %]">line:</label>
-                            <input class="column-line" id="column-line" name="[% header.cell | html %]" type="checkbox">
-                        </div>
-                    </fieldset>
-                    [% column = column + 1 %]
-                [% END %]
-            </li>
+                        <li>
+                            <label for="exclude-last">Exclude last line (Rollup): </label>
+                            <input id="exclude-last" name="chart-exclude-last" type="checkbox" />
+                        </li>
+                        [% column = 1 %]
+                        <li class="column_config_row">
+                            [% FOREACH header IN header_row %]
+                                <fieldset class="chart-column-conf" id="column_[% column | html %]">
+                                    <legend>
+                                        Column [% column | html %]
+                                        <a class="chart-column-delete" href="#" data-column="[% column | html %]">
+                                            <i class="fa fa-remove error"></i>
+                                        </a>
+                                    </legend>
+                                    <div>
+                                        <label for="y_[% column | html %]" >y:</label>
+                                        <select id="y_[% column | html %]" name="y">
+                                            <option value="[% supposed_x | html %]" selected>[% supposed_x | html %]</option>
+                                            [% FOREACH h IN header_row %]
+                                                [% IF header.cell == h.cell %]
+                                                    <option value="[% h.cell | html %]" selected>[% h.cell | html %]</option>
+                                                [% ELSE %]
+                                                    <option value="[% h.cell | html %]">[% h.cell | html %]</option>
+                                                [% END %]
+                                        [% END %]
+                                        </select>
+                                    </div>
 
-            <li>
+                                    <div class="chart-column-group">
+                                        [% i = 1 %]
+                                        <label for="group_[% column | html %]">Group:</label>
+                                        <select id="group_[% column | html %]" name="group">
+                                            [% FOREACH h IN header_row %]
+                                                [% IF i == column %]
+                                                    <option value="[% i | html %]" selected>[% i | html %]</option>
+                                                [% ELSE %]
+                                                    <option value="[% i | html %]">[% i | html %]</option>
+                                                [% END %]
+                                                [% i = i + 1 %]
+                                            [% END %]
+                                        </select>
+                                    </div>
+
+                                    <div class="chart-column-line">
+                                        <label for="line_[% column | html %]">Line:</label>
+                                        <input class="column-line" id="line_[% column | html %]" name="[% header.cell | html %]" type="checkbox" />
+                                    </div>
+                                </fieldset>
+                                [% column = column + 1 %]
+                            [% END %]
+                        </li>
+                    </ol>
+                </fieldset>
+
+                [% item = { cell = supposed_x } %]
+                [% header_row.unshift(item) | html %]
+            </div>
+            <div class="modal-footer">
                 <button id="draw-chart" class="btn btn-default">Draw</button>
-            </li>
-        </ol>
-    </fieldset>
-    [% item = { cell = supposed_x } %]
-    [% header_row.unshift(item) | html %]
-    <div id="chart"></div>
+                <button type="button" class="btn btn-default" data-dismiss="modal">Close</button>
+            </div>
+        </div>
+    </div>
 </div>
index 83ad6af..17c6654 100644 (file)
@@ -70,8 +70,7 @@
                 <a class="btn btn-default btn-sm toggle_sql" id="toggle_sql_vis" href="#" style="display:none;"><i class="fa fa-eye-slash"></i> Hide SQL code</a>
             </div>
             <div class="btn-group">
-                <a class="btn btn-default btn-sm toggle_chart_settings" id="toggle_chart_settings_hid" href="#"><i class="fa fa-eye"></i> Show chart settings</a>
-                <a class="btn btn-default btn-sm toggle_chart_settings" id="toggle_chart_settings_vis" href="#" style="display:none;"><i class="fa fa-eye-slash"></i> Hide chart settings</a>
+                <a class="btn btn-default btn-sm" href="#" data-toggle="modal" data-target="#chartModal"><i class="fa fa-bar-chart"></i> Create chart</a>
             </div>
         [% END %]
 
index 18410c2..066999f 100644 (file)
@@ -40,8 +40,8 @@
     resize:  vertical;
 }
 </style>
+[% Asset.css("css/reports.css") | $raw %]
 [% IF ( saved1 ) %]
-    [% Asset.css("css/reports.css") | $raw %]
     [% Asset.css("css/datatables.css") | $raw %]
 [% END %]
 [% Asset.css("lib/d3c3/c3.min.css") | $raw %]
@@ -697,11 +697,15 @@ canned reports and writing custom SQL reports.</p>
 
 [% IF ( execute ) %]
 <h1>[% name | html %]</h1>
-[% INCLUDE 'chart.inc' %]
 [% IF ( notes ) %]<p><span class="label">Notes:</span> [% notes | html %]</p>[% END %]
 [% IF ( unlimited_total ) %]<p><span class="label">Total number of results:</span> [% unlimited_total | html %][% IF unlimited_total > limit %] ([% limit | html %] shown)[% END %].</p>[% END %]
 <div id="sql_output" style="display:none;"><span class="label">Report SQL:</span><pre>[% sql | html %]</pre></div>
-</br>
+
+<div>
+    <a href="#" id="toggle_chart_settings_hid" class="toggle_chart_settings" style="display:none"><i class="fa fa-eye-slash"></i> Hide chart</a>
+    <a href="#" id="toggle_chart_settings_vis" class="toggle_chart_settings" style="display:none"><i class="fa fa fa-eye"></i> Show chart</a>
+</div>
+<div id="chart" class="clearfix"></div>
 
 <form action="/cgi-bin/koha/reports/guided_reports.pl" method="get" id="limitselect">
     <input type="hidden" name="phase" value="Run this report"/>
@@ -758,6 +762,9 @@ canned reports and writing custom SQL reports.</p>
     </form>
 
 [% END %]
+
+[% INCLUDE 'chart.inc' %]
+
 [% END %]
 
 [% IF ( create ) %]
@@ -1059,9 +1066,6 @@ canned reports and writing custom SQL reports.</p>
             [% IF results && !errors %]
                 $('#draw-chart').click(function() {
 
-                    var btn_text = $("#draw-chart").html();
-                    $("#draw-chart").html(_("Loading..."));
-
                     var x_elements = $('select[name="x"]').val();
                     var y_elements = [];
                     var groups = [];
@@ -1125,10 +1129,9 @@ canned reports and writing custom SQL reports.</p>
                     options.lines = lines;
 
                     chart = create_chart(kept_headers, kept_results, x_elements, y_elements, groups, options);
-                    $('#chart').prepend('<div style="font-size: 1rem; text-align: center;">' + "[% name | html %]" + '</div>');
-                    $('#download-chart').show();
-                    $("#draw-chart").html(_(btn_text));
-                    $("html, body").animate({ scrollTop: $(document).height() }, "slow");
+                    $("#download-chart,#toggle_chart_settings_hid,#chart").show();
+                    $("#toggle_chart_settings_vis").hide();
+                    $("#chartModal").modal("hide");
                 });
             [% END %]
             [% IF ( create ) %]
@@ -1267,27 +1270,25 @@ canned reports and writing custom SQL reports.</p>
                 });
             [% END %]
 
-                $(".toggle_sql").click(function(){
-                    $("#sql_output").toggle();
-                    $("#toggle_sql_hid").toggle();
-                    $("#toggle_sql_vis").toggle();
-                });
+            $(".toggle_sql").click(function(){
+                $("#sql_output").toggle();
+                $("#toggle_sql_hid").toggle();
+                $("#toggle_sql_vis").toggle();
+            });
 
-                $(".toggle_chart_settings").click(function(){
-                    $("#makechart").toggle();
-                    $("#toggle_chart_settings_hid").toggle();
-                    $("#toggle_chart_settings_vis").toggle();
-                });
+            $(".toggle_chart_settings").click(function(){
+                $("#chart, #toggle_chart_settings_hid, #toggle_chart_settings_vis").toggle();
+            });
 
-                $("#table_reports").delegate(".confirmdelete", 'click', function(){
-                    $(this).parents('tr').attr("class","warn");
-                    if(confirm(_("Are you sure you want to delete this saved report?"))){
-                        return true;
-                    } else {
-                        $(this).parents('tr').attr("class","");
-                        return false;
-                    }
-                });
+            $("#table_reports").delegate(".confirmdelete", 'click', function(){
+                $(this).parents('tr').attr("class","warn");
+                if(confirm(_("Are you sure you want to delete this saved report?"))){
+                    return true;
+                } else {
+                    $(this).parents('tr').attr("class","");
+                    return false;
+                }
+            });
 
             [% IF (create || editsql || save) %]
                 $("#select_group").change(function() {
@@ -1341,6 +1342,15 @@ canned reports and writing custom SQL reports.</p>
             $(".delete").on("click",function(){
                 return confirmDelete(MSG_CONFIRM_DELETE);
             });
+
+            $("#column_submit").submit(function() {
+                if ($("#selectedColumns option").size() < 1) {
+                    alert(_("No columns selected!"));
+                    return false;
+                }
+                $("#selectedColumns option").attr("selected", "selected");  // Select everything still in #selectedColumns
+                return true;
+            });
         });
         function addColumn() {
             $("#availableColumns option:selected").clone().appendTo("#selectedColumns").attr("selected", "selected");
@@ -1348,14 +1358,6 @@ canned reports and writing custom SQL reports.</p>
         function delColumn() {
             $("#selectedColumns option:selected").remove();
         }
-        $("#column_submit").submit(function() {
-            if ($("#selectedColumns option").size() < 1) {
-                alert(_("No columns selected!"));
-                return false;
-            }
-            $("#selectedColumns option").attr("selected", "selected");  // Select everything still in #selectedColumns
-            return true;
-        });
     </script>
 [% END %]