Bug 40896: (follow-up) Use icon wrapper elements and visibility toggling

- Wrap icons in semantic container elements for visibility toggling
- Create throttled-button.inc with Template Toolkit BLOCKs to reduce duplication
- Extract throttle logic to throttledButton.js with event delegation
- Make timeout configurable via data-throttle-timeout attribute
- Replace string-based modal generation with template element to support
  SSR

To test:
1) Follow the test plan from the previous commit
2) Additionally verify the throttled button behavior works in all locations:
   - Reports list table: "Run" button for each saved report
   - Report toolbar: "Run report" button (when viewing/editing a report)
   - Parameters form: "Run the report" button
   - Edit SQL form: "Update and run SQL" button
   - Preview SQL modal: "Run report" button (click "Preview SQL" in dropdown)
3) For each location, confirm:
   - Button shows play icon initially
   - Upon click, button becomes disabled with spinner icon
   - Button has aria-busy="true" when disabled
   - After navigation/submission, button state is handled correctly
4) Test the modal button specifically:
   - Open Preview SQL modal from any report dropdown
   - Click "Run report" in the modal
   - Verify throttle behavior works for this dynamically created button
5) Verify that the set timeout works well, I thought 120 seconds is too
   long if you go back via bfcache (back button in the browser). You'd
   have to either reload or wait 2 minutes for the button to become
   interactive again.
6) Sign off

Signed-off-by: Jake Deery <jake.deery@openfifth.co.uk>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 53cc39ae2d)
Signed-off-by: Jacob O'Mara <Jacob.omara@openfifth.co.uk>
This commit is contained in:
Paul Derscheid 2025-10-15 10:50:34 +02:00 committed by Jacob O'Mara
parent 7d71525ec5
commit 0c0b6b487e
No known key found for this signature in database
4 changed files with 156 additions and 75 deletions

View File

@ -64,9 +64,7 @@
[% IF ( showsql || execute || editsql || save_successful ) %]
[% UNLESS ( errors ) # Unless there are errors saving a report %]
<div class="btn-group">
<a id="runreport" class="btn btn-default" href="/cgi-bin/koha/reports/guided_reports.pl?id=[% id | html %]&amp;op=run"> <i class="fa fa-play"></i> Run report </a>
</div>
<div class="btn-group"> [% INCLUDE 'throttled-button.inc' element_type='link' id='runreport' href='/cgi-bin/koha/reports/guided_reports.pl?id=' _ id _ '&op=run' classes='btn btn-default' label=t('Run report') %] </div>
[% IF templates.count %]
<div class="btn-group">
<button type="button" class="btn btn-default dropdown-toggle" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false"> <i class="fa fa-code"></i> Run with template </button>

View File

@ -0,0 +1,23 @@
[%# USAGE [ INCLUDE 'throttled-button.inc' element_type='button' classes='btn btn-primary' label=t('Run report') attrs={ name => 'op', value => 'cud-run' } ] %]
[% DEFAULT element_type = 'link' %]
[% DEFAULT ready_icon = 'fa fa-play' %]
[% DEFAULT active_icon = 'fa fa-spinner fa-spin' %]
[% DEFAULT classes = 'btn btn-default' %]
[% DEFAULT href = '#' %]
[%# Common attributes shared by both button and link %]
[% BLOCK common_attrs %]class="[% classes | html %]"[% IF id %]id="[% id | html %]"[% END %] data-state="ready" data-throttled-button [% IF attrs %][% FOREACH key IN attrs.keys %][% key | html %]="[% attrs.$key | html %]"[% END %][% END %][% END %]
[%# Inner content shared by both button and link %]
[% BLOCK inner_content %]
<span class="button-icon-ready"><i class="[% ready_icon | html %]"></i></span>
<span class="button-icon-running" style="display: none;"><i class="[% active_icon | html %]"></i></span>
[% UNLESS icon_only %][% label | html %][% END %]
[% END %]
[% IF element_type == 'button' -%]
<button type="submit" [% PROCESS common_attrs %]> [% PROCESS inner_content %] </button>
[%- ELSE -%]
<a [% PROCESS common_attrs %] href="[% href | html %]"> [% PROCESS inner_content %] </a>
[%- END %]

View File

@ -379,8 +379,13 @@
<td>
<div class="btn-group dropup">
[%# There should be no space between these two buttons, it would render badly %]
<a class="btn btn-default btn-xs" role="button" href="/cgi-bin/koha/reports/guided_reports.pl?id=[% savedreport.id | html %]&amp;op=run"><i class="fa fa-play"></i> Run</a
><a class="btn btn-default btn-xs dropdown-toggle" id="reportactions[% savedreport.id | html %]" role="button" data-bs-toggle="dropdown" href="#"></a>
[% INCLUDE 'throttled-button.inc' element_type='link' href='/cgi-bin/koha/reports/guided_reports.pl?id=' _ savedreport.id _ '&op=run' classes='btn btn-default btn-xs' label=t('Run') attrs={ role => 'button' } %]<a
class="btn btn-default btn-xs dropdown-toggle"
id="reportactions[% savedreport.id | html %]"
role="button"
data-bs-toggle="dropdown"
href="#"
></a>
<ul class="dropdown-menu" role="menu" aria-labelledby="reportactions[% savedreport.id | html %]">
<li
><a class="dropdown-item" href="/cgi-bin/koha/reports/guided_reports.pl?id=[% savedreport.id | uri %]&amp;op=show"
@ -930,7 +935,7 @@
[% IF ( template_id ) %]
<input type="hidden" name="template" value="[% template_id | html %]" />
[% END %]
<button type="submit" class="btn btn-primary"><i class="fa fa-play"></i> Run the report</button>
[% INCLUDE 'throttled-button.inc' element_type='button' classes='btn btn-primary' label=t('Run the report') %]
</fieldset>
</form>
[% END # / IF ( auth_val_error ) %]
@ -1575,7 +1580,7 @@
<fieldset class="action">
<button class="btn btn-primary" type="submit" name="op" value="cud-update_sql">Update SQL</button>
<button class="btn btn-default" type="submit" name="op" value="cud-update_and_run_sql">Update and run SQL</button>
[% INCLUDE 'throttled-button.inc' element_type='button' classes='btn btn-default' label=t('Update and run SQL') attrs={ name => 'op', value => 'cud-update_and_run_sql' } %]
<a href="/cgi-bin/koha/reports/guided_reports.pl?op=list" class="cancel">Cancel</a>
</fieldset>
</form>
@ -1738,6 +1743,7 @@
[% Asset.js( "lib/codemirror/highlight.js" ) | $raw %]
[% Asset.css("lib/codemirror/highlight.css") | $raw %]
[% Asset.js( "js/mana.js" ) | $raw %]
[% Asset.js( "js/throttledButton.js" ) | $raw %]
[% INCLUDE 'select2.inc' %]
<script>
// if the report param form has multiselects override default form submission
@ -2005,18 +2011,6 @@
}
}
const toggleReportRunButton = (element => {
if ($(element).attr('disabled') === undefined) {
$(element).attr('disabled', 'disabled');
$(element).addClass('disabled');
$(element).find('i.fa-play').first().replaceWith('<i class=\"fa fa-spinner fa-spin\" style=\"--fa-animation-duration: 2s;\"></i>');
} else {
$(element).removeAttr('disabled');
$(element).removeClass('disabled');
$(element).find('i.fa-spinner').first().replaceWith('<i class=\"fa fa-play\"></i>');
}
});
$(document).ready(function(){
var activeTab = localStorage.getItem("sql_reports_activetab");
@ -2493,40 +2487,6 @@
$(selectField).select2();
});
$('a[href*="/cgi-bin/koha/reports/guided_reports.pl"]').each((idx, element) => {
const params = new URLSearchParams($(element).attr('href'));
const op = params.get('op');
if (op !== 'run') {
return;
}
$(element).on('click', event => {
event.preventDefault();
const href = $(element).attr('href');
toggleReportRunButton(element);
setTimeout(() => {
toggleReportRunButton(element);
}, 120000);
window.location.href = href;
return false;
});
});
$('#report_param_form').on('submit', event => {
const button = $('button[type="submit"]');
if (button.length < 1) {
return;
}
toggleReportRunButton(button);
setTimeout(() => {
toggleReportRunButton(button);
}, 120000);
});
});
$("#toggle_auto_links").on("click", function(e){
@ -2669,33 +2629,38 @@
// Adapted from https://gist.github.com/jnormore/7418776
function previewSql(reportid) {
var yes_label = "";
var no_label = "";
var message = $("#previewSql" + reportid ).val();
var title = $("#previewSql" + reportid ).data("title");
if( $("#preview-sql-modal").length > 0) {
$("#preview-sql-modal").remove();
}
$("body").append('<div id="preview-sql-modal" tabindex="-1" role="dialog" aria-hidden="true" class="modal">\
<div class="modal-dialog modal-xl">\
<div class="modal-content">\
<div class="modal-header" style="min-height:40px;">\
<h1 class="modal-title">' + title + '</h1>\
<button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="' + _("Close") + '"></button>\
</div>\
<div class="modal-body"><textarea id="code' + reportid + '">' + message + '</textarea>\
</div>\
<div class="modal-footer">\
<a id="preview-modal-editreport" class="btn btn-default" href="/cgi-bin/koha/reports/guided_reports.pl?id=' + reportid + '&amp;op=edit_form"><i class="fa-solid fa-pencil" aria-hidden="true"></i> ' + _("Edit") + '</a>\
<a id="preview-modal-duplicate" class="btn btn-default" href="/cgi-bin/koha/reports/guided_reports.pl?op=duplicate&amp;id=' + reportid + '"><i class="fa fa-copy" aria-hidden="true"></i> ' + _("Duplicate") + '</a>\
<a id="preview-modal-duplicate" class="btn btn-default" href="/cgi-bin/koha/tools/scheduler.pl?id=' + reportid + '"><i class="fa-solid fa-clock" aria-hidden="true"></i> ' + _("Schedule") + '</a>\
<a class="btn btn-default submit-form-link" href="#" data-id="' + reportid + '" data-action="guided_reports.pl" data-method="post" data-op="cud-delete" data-confirmation-msg="' + _("Are you sure you want to delete this report?") + '"><i class="fa fa-trash-can" aria-hidden="true"></i> ' + _("Delete") + '</a>\
<a id="preview-modal-runreport" class="btn btn-default" href="/cgi-bin/koha/reports/guided_reports.pl?id=' + reportid + '&amp;op=run"><i class="fa fa-play" aria-hidden="true"></i> ' + _("Run report") + '</a>\
<a href="#" id="preview-sql-modal-cancel" data-bs-dismiss="modal" class="btn btn-default"><i class="fa fa-times" aria-hidden="true"></i> ' + _("Close") + '</a>\
</div>\
</div>\
</div>\
</div>');
// Clone the template content
var template = document.getElementById('preview-sql-modal-template');
var clone = document.importNode(template.content, true);
// Fill in the placeholders
clone.querySelector('[data-placeholder="title"]').textContent = title;
Object.assign(clone.querySelector('[data-placeholder="code"]'), {
id: 'code' + reportid,
textContent: message
});
clone.querySelector('[data-placeholder="edit-href"]').href = '/cgi-bin/koha/reports/guided_reports.pl?id=' + encodeURIComponent(reportid) + '&op=edit_form';
clone.querySelector('[data-placeholder="duplicate-href"]').href = '/cgi-bin/koha/reports/guided_reports.pl?op=duplicate&id=' + encodeURIComponent(reportid);
clone.querySelector('[data-placeholder="schedule-href"]').href = '/cgi-bin/koha/tools/scheduler.pl?id=' + encodeURIComponent(reportid);
Object.assign(clone.querySelector('[data-placeholder="delete-attrs"]').dataset, {
id: reportid,
action: 'guided_reports.pl',
method: 'post',
op: 'cud-delete'
});
clone.querySelector('[data-placeholder="run-href"]').href = '/cgi-bin/koha/reports/guided_reports.pl?id=' + encodeURIComponent(reportid) + '&op=run';
document.body.appendChild(clone);
$("#preview-sql-modal").on("shown.bs.modal", function(){
CodeMirror.fromTextArea( document.getElementById("code" + reportid ), {
@ -2710,6 +2675,30 @@
</script>
[% END %]
<template id="preview-sql-modal-template">
<div id="preview-sql-modal" tabindex="-1" role="dialog" aria-hidden="true" class="modal">
<div class="modal-dialog modal-xl">
<div class="modal-content">
<div class="modal-header" style="min-height:40px;">
<h1 class="modal-title" data-placeholder="title"></h1>
<button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="[% t('Close') | html %]"></button>
</div>
<div class="modal-body"><textarea data-placeholder="code"></textarea></div>
<div class="modal-footer">
<a id="preview-modal-editreport" class="btn btn-default" data-placeholder="edit-href"><i class="fa-solid fa-pencil" aria-hidden="true"></i> Edit</a>
<a id="preview-modal-duplicate" class="btn btn-default" data-placeholder="duplicate-href"><i class="fa fa-copy" aria-hidden="true"></i> Duplicate</a>
<a id="preview-modal-schedule" class="btn btn-default" data-placeholder="schedule-href"><i class="fa-solid fa-clock" aria-hidden="true"></i> Schedule</a>
<a class="btn btn-default submit-form-link" href="#" data-placeholder="delete-attrs" data-confirmation-msg="[% t('Are you sure you want to delete this report?') | html %]"
><i class="fa fa-trash-can" aria-hidden="true"></i> Delete</a
>
[% INCLUDE 'throttled-button.inc' element_type='link' id='preview-modal-runreport' classes='btn btn-default' label=t('Run report') attrs={ 'data-placeholder' => 'run-href' } %]
<a href="#" id="preview-sql-modal-cancel" data-bs-dismiss="modal" class="btn btn-default"><i class="fa fa-times" aria-hidden="true"></i> Close</a>
</div>
</div>
</div>
</div>
</template>
[% INCLUDE 'intranet-bottom.inc' %]
[% BLOCK group_and_subgroup_selection %]

View File

@ -0,0 +1,71 @@
(() => {
const DEFAULT_TIMEOUT = 3000;
const toggleButton = element => {
const isDisabled = element.hasAttribute("disabled");
const readyIcon = element.querySelector(".button-icon-ready");
const runningIcon = element.querySelector(".button-icon-running");
if (!readyIcon || !runningIcon) {
return;
}
if (!isDisabled) {
element.setAttribute("disabled", "disabled");
element.setAttribute("aria-busy", "true");
element.classList.add("disabled");
element.dataset.state = "running";
readyIcon.style.display = "none";
runningIcon.style.display = "";
} else {
element.removeAttribute("disabled");
element.removeAttribute("aria-busy");
element.classList.remove("disabled");
element.dataset.state = "ready";
readyIcon.style.display = "";
runningIcon.style.display = "none";
}
};
// Use event delegation to handle both static and dynamically created links
document.addEventListener("click", event => {
const link = event.target.closest("a[data-throttled-button]");
if (link) {
event.preventDefault();
const href = link.getAttribute("href");
toggleButton(link);
window.location.href = href;
}
});
// Handle button elements within forms
document.addEventListener("submit", event => {
const submitter = event.submitter;
if (submitter && submitter.hasAttribute("data-throttled-button")) {
const timeoutValue = parseInt(
submitter.dataset.throttleTimeout,
10
);
const timeout = isNaN(timeoutValue)
? DEFAULT_TIMEOUT
: timeoutValue;
requestAnimationFrame(() => {
toggleButton(submitter);
setTimeout(() => toggleButton(submitter), timeout);
});
}
});
// Reset button state when page is restored from bfcache
window.addEventListener("pageshow", event => {
if (event.persisted) {
document
.querySelectorAll("[data-throttled-button][disabled]")
.forEach(button => {
toggleButton(button);
});
}
});
})();