From c5e5bc404d2c2ef9ecc5bf23ebfb2a83801dc3cf Mon Sep 17 00:00:00 2001 From: Paul Derscheid Date: Fri, 15 Aug 2025 11:00:03 +0000 Subject: [PATCH] Bug 40656: Refactor bookings/list.tt to use better patterns and improve code organization This patch aims to improve bookings/list.tt: - Converting filter attributes to boolean data attributes (presence-based) - Creating a reusable table-filters.inc for filter controls - Moving inline styles to proper SCSS files for better caching and separation of concerns - Fixing a runtime error when cancelling bookings from the timeline - Improving JavaScript readability with helper functions - Addressing the QA complaints from Bug 36789: - Cleaning up the long lines for the button elements by moving them into template literals - Using the format method instead of direct interpolation - Using escape_str for all values received from the API. Test plan: 1. Apply the patch and compile css 2. Navigate to a biblio record with bookings (Catalog > Search > View record > Bookings) 3. Verify the filter buttons (Exclude expired/cancelled) appear correctly 4. Click each filter button and verify: - The button text toggles between "Include" and "Exclude" - The table updates to show/hide the appropriate bookings - The status column appears when any items are filtered 5. In the timeline view: - Verify cancelled bookings show with a strikethrough pattern - Try to cancel a booking by clicking the X on a timeline item - Confirm the cancel modal opens without JavaScript errors 6. Check that the table actions (Edit, Cancel) work correctly 7. Verify no inline styles remain in the page source 8. Confirm the filter styles are properly loaded from compiled CSS Signed-off-by: Martin Renvoize --- .../intranet-tmpl/prog/css/src/bookings.scss | 13 ++ .../prog/css/src/staff-global.scss | 14 ++ .../prog/en/includes/table-filters.inc | 30 ++++ .../prog/en/modules/bookings/list.tt | 162 ++++++++++-------- 4 files changed, 145 insertions(+), 74 deletions(-) create mode 100644 koha-tmpl/intranet-tmpl/prog/css/src/bookings.scss create mode 100644 koha-tmpl/intranet-tmpl/prog/en/includes/table-filters.inc diff --git a/koha-tmpl/intranet-tmpl/prog/css/src/bookings.scss b/koha-tmpl/intranet-tmpl/prog/css/src/bookings.scss new file mode 100644 index 00000000000..3f72fbf488f --- /dev/null +++ b/koha-tmpl/intranet-tmpl/prog/css/src/bookings.scss @@ -0,0 +1,13 @@ +#bookings-timeline { + .vis-item.vis-range { + &.cancelled { + background: repeating-linear-gradient( + 135deg, + rgba(211, 211, 211, 0.5) 0, + rgba(211, 211, 211, 0.5) 10px, + transparent 10px, + transparent 20px + ); + } + } +} \ No newline at end of file diff --git a/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss b/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss index a43aa86c15a..6a12efefc61 100644 --- a/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss +++ b/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss @@ -4894,3 +4894,17 @@ div .suggestion_note { } } } + +[class*="filters-"] { + [data-filtered] span:nth-child(2) { + display: none; + } + + a:not([data-filtered]) span:nth-child(1) { + display: none; + } + + a { + cursor: pointer; + } +} diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/table-filters.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/table-filters.inc new file mode 100644 index 00000000000..714ce3c1d78 --- /dev/null +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/table-filters.inc @@ -0,0 +1,30 @@ +[%# TABLE FILTER CONTROLS USAGE %] +[%# [ INCLUDE 'table-filters.inc' %] +[%# filters = [ %] +[%# { id = 'expired', label_show = t('Show expired'), label_hide = t('Hide expired') }, %] +[%# { id = 'cancelled', label_show = t('Show cancelled'), label_hide = t('Hide cancelled') }, %] +[%# { id = 'completed', label_show = t('Show completed'), label_hide = t('Hide completed') } %] +[%# ] %] +[%# filter_class = 'bookings' # Optional, defaults to 'filters' %] +[%# ] %] +[%# %] +[%# Parameters: %] +[%# - filters: Array of filter objects with: %] +[%# - id: Unique identifier for the filter %] +[%# - label_show: Text when filter shows items %] +[%# - label_hide: Text when filter hides items %] +[%# - active: Boolean - filter starts active (optional, defaults to true) %] +[%# - filter_class: CSS class prefix (optional, defaults to 'filters') %] + +[% IF filters && filters.size > 0 %] + [% filter_class = filter_class || 'filters' %] + +
+ [% FOREACH filter IN filters %] + + [% filter.label_show | html %] + [% filter.label_hide | html %] + + [% END %] +
+[% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/bookings/list.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/bookings/list.tt index f9e4c193785..f3341d5b714 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/bookings/list.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/bookings/list.tt @@ -12,13 +12,7 @@ [% END %] [% INCLUDE 'doc-head-close.inc' %] - +[% Asset.css("css/bookings.css") | $raw %] @@ -47,10 +41,14 @@

Bookings for [% INCLUDE 'biblio-title-head.inc' %]

-
- Show expired - Show cancelled -
+ [% + INCLUDE 'table-filters.inc' + filters = [ + { id = 'expired', label_show = t('Include expired'), label_hide = t('Exclude expired') }, + { id = 'cancelled', label_show = t('Include cancelled'), label_hide = t('Exclude cancelled') }, + ] + filter_class = 'bookings' + %]
@@ -124,6 +122,7 @@ start: dayjs(booking.start_date).toDate(), end: dayjs(booking.end_date).toDate(), content: !isActive ? `${patronContent}` : patronContent, + className: booking.status === "cancelled" ? "cancelled" : "", [% IF CAN_user_circulate_manage_bookings %] editable: booking.status !== "cancelled" ? { remove: true, updateTime: true } : false, [% ELSE %] @@ -134,17 +133,6 @@ }); } - const cancelledItems = document.querySelectorAll('#bookings-timeline .vis-item.vis-range.cancelled'); - cancelledItems.forEach(item => { - item.style.background = 'repeating-linear-gradient(' + - '135deg,' + - 'rgba(211, 211, 211, 0.5) 0,' + - 'rgba(211, 211, 211, 0.5) 10px,' + - 'transparent 10px,' + - 'transparent 20px' + - ');'; - }); - var container = document.getElementById('bookings-timeline'); var options = { stack: true, @@ -201,7 +189,9 @@ }); }, onRemove: function (item, callback) { - $('#cancelBookingModal').modal('show', $(''.format(row.booking_id, biblionumber, row.item_id, row.patron_id, row.pickup_library_id, row.start_date, row.end_date, row.item.item_type_id, _("Edit")); - result += ''.format(row.booking_id, _("Cancel")); + if (!is_readonly) { + result += ` + + `.format( + escape_str(row.booking_id), + escape_str(biblionumber), + escape_str(row.item_id), + escape_str(row.patron_id), + escape_str(row.pickup_library_id), + escape_str(row.start_date), + escape_str(row.end_date), + escape_str(row.item.item_type_id), + _("Edit") + ); + result += ` + + `.format( + escape_str(row.booking_id), + _("Cancel") + ); } [% END %] return result; @@ -386,51 +440,11 @@ }] }, undefined, 0, additional_filters); - document - .getElementById("expired_filter") - .addEventListener("click", (e) => - handleFilter( - e, - { active: _("Hide expired"), inactive: _("Show expired") }, - filterStates, - ), - ); - - document - .getElementById("cancelled_filter") - .addEventListener("click", (e) => - handleFilter( - e, - { active: _("Hide cancelled"), inactive: _("Show cancelled") }, - filterStates, - ), - ); - $("#cancellation-reason").comboBox({ displayProperty: 'name', placeholder: _("Select or type a reason"), }); - function handleFilter(e, text, filterStates) { - const target = e.target; - const { filter } = target.dataset; - if (target.classList.contains("filtered")) { - filterStates[filter] = false; - target.innerHTML = ` ${text.inactive}`; - } else { - filterStates[filter] = true; - target.innerHTML = ` ${text.active}`; - } - - bookings_table.DataTable().ajax.reload(() => { - bookings_table - .DataTable() - .column("status:name") - .visible(filterStates[filter], false); - }); - - target.classList.toggle("filtered"); - } }); [% END %] -- 2.51.0