From 0e07980c884e0ec2480496250ce958df9b5a9c24 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Sat, 3 Jan 2026 07:34:09 +0000 Subject: [PATCH] Bug 37707: (follow-up) Add textual feedback for booking lead/trail period conflicts This patch adds a feedback message container below the calendar that provides contextual information about lead/trail period requirements and conflicts, improving accessibility and user understanding. The feedback updates dynamically on hover, showing: - Info messages when hovering selectable dates - Error messages when hovering dates disabled due to conflicts - Specific explanations distinguishing between conflict types (e.g., new lead vs existing trail, new trail vs existing lead) - Multiple conflict messages when both lead and trail periods conflict - Specific messages for past dates and insufficient lead time Key improvements: - Standardized terminology consistently uses "lead period" and "trail period" - Proper internationalization using __() for all user-facing strings - DRY code with detectPeriodConflict() helper function - Styling moved from inline JavaScript to SCSS using Bootstrap alert classes - Empty/zero periods are handled gracefully (not mentioned in messages) - Accurate conflict detection prioritizes what the hovered date IS over what it conflicts with This complements the visual color indicators by providing explicit textual explanations, improving accessibility and user understanding. Test plan: 1. Apply all patches and run: yarn build 2. Configure a bookable item with 2-day lead and 3-day trail periods 3. Create an existing booking for January 10-15 (protected: Jan 8-18) 4. Open the booking calendar and observe feedback area below calendar 5. Test past date and insufficient lead time messages: - Hover on today's date * Verify message: "Cannot select: Date is in the past" - Hover on tomorrow (with 2-day lead period) * Verify message: "Cannot select: Insufficient lead time (2 days required before start)" - Hover on the day after tomorrow * Verify message: "Cannot select: Insufficient lead time (2 days required before start)" * Should NOT show "conflicts with existing booking" if no booking exists 6. Test existing booking conflict messages: - Hover over Jan 12 (part of existing booking) * Verify message: "Cannot select: This date is part of an existing booking" - Hover over Jan 8 (existing booking's lead period) * Verify message: "Cannot select: This date is part of an existing booking's lead period" - Hover over Jan 16 (existing booking's trail period) * Verify message: "Cannot select: This date is part of an existing booking's trail period" 7. Test lead/trail period conflict messages: - Try to select Jan 19-20 as booking dates (2-day lead = Jan 17-18) - Hover over Jan 19 when it appears disabled * Verify error message: "Cannot select: Lead period (2 days before start) conflicts with existing booking's trail period" 8. Test info messages for valid dates: - Hover over Jan 21 (no conflicts, with lead period configured) * Verify info message: "Selecting start date. Lead period: 2 days before start" - Configure booking with NO lead period (0 or blank) - Hover over a valid date * Verify message: "Selecting start date" (no mention of lead period) 9. Test multiple conflict messages: - Position calendar to hover between two bookings where both lead and trail conflict * Verify message shows both conflicts separated by semicolon * Example: "Cannot select: lead period conflicts with existing booking's trail period; trail period conflicts with existing booking's lead period" 10. Verify all feedback messages: - Are properly translated (check .po files if applicable) - Use consistent "lead period" and "trail period" terminology - Are clear and helpful for librarians - Update color coding (red for errors, blue for info) --- .../intranet-tmpl/prog/css/src/bookings.scss | 2 +- .../prog/css/src/staff-global.scss | 13 + .../prog/js/modals/place_booking.js | 425 ++++++++++++++++-- 3 files changed, 398 insertions(+), 42 deletions(-) diff --git a/koha-tmpl/intranet-tmpl/prog/css/src/bookings.scss b/koha-tmpl/intranet-tmpl/prog/css/src/bookings.scss index 3f72fbf488f..0f9a7da6fea 100644 --- a/koha-tmpl/intranet-tmpl/prog/css/src/bookings.scss +++ b/koha-tmpl/intranet-tmpl/prog/css/src/bookings.scss @@ -10,4 +10,4 @@ ); } } -} \ 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 a75fca56e89..8e3dec110a8 100644 --- a/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss +++ b/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss @@ -1103,6 +1103,19 @@ $leadClashTrail: #e5d4f5 !default; } } +// Booking conflict feedback message styles +.booking-conflict-feedback { + // Override Bootstrap's default alert padding and sizing for compact display + padding: 0.5rem 0.75rem; + margin-top: 0.5rem; + margin-bottom: 0 !important; // Override Bootstrap's default 1rem margin-bottom + min-height: 1.25rem; + text-align: center; + + // Bootstrap's .alert-danger, .alert-warning, and .alert-info classes + // provide the color schemes via CSS variables when combined with .alert +} + .input-warning { background-color: #ff9; border-color: #900; diff --git a/koha-tmpl/intranet-tmpl/prog/js/modals/place_booking.js b/koha-tmpl/intranet-tmpl/prog/js/modals/place_booking.js index dc04dc17d49..c1dcf5ec3f8 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/modals/place_booking.js +++ b/koha-tmpl/intranet-tmpl/prog/js/modals/place_booking.js @@ -922,6 +922,17 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { ); } + // Create feedback message container below the calendar + let feedbackDiv = periodPicker.calendarContainer.querySelector( + ".booking-conflict-feedback" + ); + if (!feedbackDiv) { + feedbackDiv = document.createElement("div"); + feedbackDiv.className = + "booking-conflict-feedback alert d-none"; + periodPicker.calendarContainer.appendChild(feedbackDiv); + } + // Add hints for days before the start range and after the end range periodPicker.calendarContainer.addEventListener( "mouseover", @@ -941,9 +952,15 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { const leadStart = startDate ? startDate.subtract(leadDays, "day") : hoverDate.subtract(leadDays, "day"); - const leadEnd = startDate ? startDate : hoverDate; - const trailStart = hoverDate; - const trailEnd = hoverDate.add(trailDays, "day"); + const leadEnd = startDate + ? startDate.subtract(1, "day") + : hoverDate.subtract(1, "day"); + const trailStart = startDate + ? hoverDate.add(1, "day") + : hoverDate.add(1, "day"); + const trailEnd = startDate + ? hoverDate.add(trailDays, "day") + : hoverDate.add(trailDays, "day"); // BIDIRECTIONAL ENHANCEMENT: Collect closest bookings for visual feedback // and check for mathematical conflicts in a single pass @@ -956,6 +973,18 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { let leadDisable = false; let trailDisable = false; + // Track conflict reasons for messaging + let leadConflictReason = { + withTrail: false, + withLead: false, + withBooking: false, + }; + let trailConflictReason = { + withTrail: false, + withLead: false, + withBooking: false, + }; + bookings.forEach(booking => { // Skip if we're editing this booking if ( @@ -1000,8 +1029,9 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { "day" ); - // Check if new booking's LEAD period overlaps with existing booking's TRAIL period + // Check if new booking's LEAD period overlaps with existing booking if (!periodPicker.selectedDates[0]) { + // Check overlap with existing booking's trail period if ( leadStart.isSameOrBefore( existingTrailEnd @@ -1011,11 +1041,31 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { ) ) { leadDisable = true; + leadConflictReason.withTrail = true; + } + // Check overlap with existing booking's lead period + else if ( + leadStart.isSameOrBefore( + existingLeadEnd + ) && + leadEnd.isSameOrAfter(existingLeadStart) + ) { + leadDisable = true; + leadConflictReason.withLead = true; + } + // Check overlap with existing booking itself + else if ( + leadStart.isSameOrBefore(bookingEnd) && + leadEnd.isSameOrAfter(bookingStart) + ) { + leadDisable = true; + leadConflictReason.withBooking = true; } } - // Check if new booking's TRAIL period overlaps with existing booking's LEAD period + // Check if new booking's TRAIL period overlaps with existing booking if (periodPicker.selectedDates[0]) { + // Check overlap with existing booking's lead period if ( trailStart.isSameOrBefore( existingLeadEnd @@ -1025,6 +1075,27 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { ) ) { trailDisable = true; + trailConflictReason.withLead = true; + } + // Check overlap with existing booking's trail period + else if ( + trailStart.isSameOrBefore( + existingTrailEnd + ) && + trailEnd.isSameOrAfter( + existingTrailStart + ) + ) { + trailDisable = true; + trailConflictReason.withTrail = true; + } + // Check overlap with existing booking itself + else if ( + trailStart.isSameOrBefore(bookingEnd) && + trailEnd.isSameOrAfter(bookingStart) + ) { + trailDisable = true; + trailConflictReason.withBooking = true; } } @@ -1058,6 +1129,7 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { } }); + // Work through all days in view and add classes appropraitely based on hovered date periodPicker.calendarContainer .querySelectorAll(".flatpickr-day") .forEach(function (dayElem) { @@ -1085,35 +1157,46 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { "existingBookingTrailEnd" ); - // Apply new booking's lead/trail period classes - dayElem.classList.toggle( - "leadRangeStart", - elemDate.isSame(leadStart) - ); - dayElem.classList.toggle( - "leadRange", - elemDate.isSameOrAfter(leadStart) && - elemDate.isBefore(leadEnd) - ); - dayElem.classList.toggle( - "leadRangeEnd", - elemDate.isSame(leadEnd) - ); - dayElem.classList.toggle( - "trailRangeStart", - elemDate.isSame(trailStart) - ); - dayElem.classList.toggle( - "trailRange", - elemDate.isAfter(trailStart) && - elemDate.isSameOrBefore(trailEnd) - ); - dayElem.classList.toggle( - "trailRangeEnd", - elemDate.isSame(trailEnd) - ); + // Apply proposed booking's lead/trail period classes + // Only apply lead classes if lead period > 0 + if (leadDays > 0) { + dayElem.classList.toggle( + "leadRangeStart", + elemDate.isSame(leadStart) + ); + dayElem.classList.toggle( + "leadRange", + elemDate.isSameOrAfter(leadStart) && + elemDate.isSameOrBefore(leadEnd) + ); + dayElem.classList.toggle( + "leadRangeEnd", + elemDate.isSame(leadEnd) + ); + } - // BIDIRECTIONAL: Show closest existing booking's trail period + // Only apply trail classes if trail period > 0 + if (trailDays > 0) { + dayElem.classList.toggle( + "trailRangeStart", + elemDate.isSame(trailStart) + ); + dayElem.classList.toggle( + "trailRange", + elemDate.isSameOrAfter( + trailStart + ) && + elemDate.isSameOrBefore( + trailEnd + ) + ); + dayElem.classList.toggle( + "trailRangeEnd", + elemDate.isSame(trailEnd) + ); + } + + // Show closest preceding existing booking's trail period if (closestBeforeBooking && trailDays > 0) { const existingTrailStart = closestBeforeBooking.end.add( @@ -1159,7 +1242,7 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { } } - // BIDIRECTIONAL: Show closest existing booking's lead period + // Show closest following existing booking's lead period if (closestAfterBooking && leadDays > 0) { const existingLeadStart = closestAfterBooking.start.subtract( @@ -1203,7 +1286,7 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { } } - // Check for conflicts with flatpickr-disabled dates (original behavior) + // Check for conflicts with flatpickr-disabled dates if ( dayElem.classList.contains( "flatpickr-disabled" @@ -1212,12 +1295,15 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { if ( !periodPicker.selectedDates[0] && elemDate.isSameOrAfter(leadStart) && - elemDate.isBefore(leadEnd) + elemDate.isSameOrBefore(leadEnd) ) { leadDisable = true; } if ( - elemDate.isAfter(trailStart) && + periodPicker.selectedDates[0] && + elemDate.isSameOrAfter( + trailStart + ) && elemDate.isSameOrBefore(trailEnd) ) { // Only consider this a conflict if the disabled date is within the max date range @@ -1238,7 +1324,7 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { } } - // BIDIRECTIONAL: Check for conflicts with existing booking's trail period + // Check for conflicts with existing booking's trail period if ( !periodPicker.selectedDates[0] && dayElem.classList.contains( @@ -1248,21 +1334,24 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { // New booking's lead period overlaps with existing booking's trail if ( elemDate.isSameOrAfter(leadStart) && - elemDate.isBefore(leadEnd) + elemDate.isSameOrBefore(leadEnd) ) { leadDisable = true; } } - // BIDIRECTIONAL: Check for conflicts with existing booking's lead period + // Check for conflicts with existing booking's lead period if ( + periodPicker.selectedDates[0] && dayElem.classList.contains( "existingBookingLead" ) ) { // New booking's trail period overlaps with existing booking's lead if ( - elemDate.isAfter(trailStart) && + elemDate.isSameOrAfter( + trailStart + ) && elemDate.isSameOrBefore(trailEnd) ) { trailDisable = true; @@ -1278,6 +1367,25 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { ); }); + // Additional check for hovering directly on existing booking's lead/trail periods + // If hovering on an existing booking's lead period when selecting start date, block selection + if ( + !periodPicker.selectedDates[0] && + target.classList.contains("existingBookingLead") + ) { + leadDisable = true; + } + + // If hovering on an existing booking's trail period when selecting end date, block selection + if ( + periodPicker.selectedDates[0] && + target.classList.contains( + "existingBookingTrail" + ) + ) { + trailDisable = true; + } + if (leadDisable) { target.classList.add("leadDisable"); } @@ -1291,6 +1399,241 @@ $("#placeBookingModal").on("show.bs.modal", function (e) { true ); } + + // Update feedback message + const feedbackDiv = + periodPicker.calendarContainer.querySelector( + ".booking-conflict-feedback" + ); + if (feedbackDiv) { + let message = ""; + let messageType = "info"; // info, warning, error + + // Determine what the hovered date is (needed for both error and info messages) + const today = dayjs().startOf("day"); + const isDisabled = + target.classList.contains( + "flatpickr-disabled" + ); + const isInExistingLead = + target.classList.contains( + "existingBookingLead" + ); + const isInExistingTrail = + target.classList.contains( + "existingBookingTrail" + ); + + // Generate appropriate feedback messages based on conflicts + if (leadDisable || trailDisable) { + messageType = "error"; + + // When selecting START date (no date selected yet) + if (!periodPicker.selectedDates[0]) { + // Check direct state first (what IS this date?) + if (hoverDate.isBefore(today)) { + message = __( + "Cannot select: date is in the past" + ); + } else if (isDisabled) { + message = __( + "Cannot select: this date is part of an existing booking" + ); + } else if (isInExistingLead) { + message = __( + "Cannot select: this date is part of an existing booking's lead period" + ); + } else if (isInExistingTrail) { + message = __( + "Cannot select: this date is part of an existing booking's trail period" + ); + } + // Then check calculated lead period conflicts + else if ( + leadDays > 0 && + leadStart.isSameOrBefore(today) + ) { + message = + __("Cannot select") + + ": " + + __( + "insufficient lead time (%s days required before start)" + ).format(leadDays); + } else if (leadDays > 0) { + // Use mathematical conflict detection (works across month boundaries) + if (leadConflictReason.withTrail) { + message = + __("Cannot select") + + ": " + + __( + "lead period (%s days before start) conflicts with an existing booking's trail period" + ).format(leadDays); + } else if ( + leadConflictReason.withLead + ) { + message = + __("Cannot select") + + ": " + + __( + "lead period (%s days before start) conflicts with an existing booking's lead period" + ).format(leadDays); + } else if ( + leadConflictReason.withBooking + ) { + message = + __("Cannot select") + + ": " + + __( + "lead period (%s days before start) conflicts with an existing booking" + ).format(leadDays); + } + } else { + message = __( + "Cannot select: conflicts with an existing booking" + ); + } + } + // When selecting END date (start date already selected) + else if (periodPicker.selectedDates[0]) { + // Check direct state first (what IS this date?) + if (isDisabled) { + message = __( + "Cannot select: this date is part of an existing booking" + ); + } else if (isInExistingLead) { + message = __( + "Cannot select: this date is part of an existing booking's lead period" + ); + } else if (isInExistingTrail) { + message = __( + "Cannot select: this date is part of an existing booking's trail period" + ); + } + // Then check calculated trail period conflicts + else if (trailDays > 0) { + // Use mathematical conflict detection (works across month boundaries) + if (trailConflictReason.withLead) { + message = + __("Cannot select") + + ": " + + __( + "trail period (%s days after return) conflicts with an existing booking's lead period" + ).format(trailDays); + } else if ( + trailConflictReason.withTrail + ) { + message = + __("Cannot select") + + ": " + + __( + "trail period (%s days after return) conflicts with an existing booking's trail period" + ).format(trailDays); + } else if ( + trailConflictReason.withBooking + ) { + message = + __("Cannot select") + + ": " + + __( + "trail period (%s days after return) conflicts with an existing booking" + ).format(trailDays); + } + } else { + message = __( + "Cannot select: conflicts with existing an booking" + ); + } + } + } else { + // Show helpful info messages when no conflicts + if (!periodPicker.selectedDates[0]) { + // When selecting start date, show both lead and trail info + if (leadDays > 0 && trailDays > 0) { + message = + __("Selecting start date") + + ". " + + __( + "Lead period: %s days before start" + ).format(leadDays) + + ". " + + __( + "Trail period: %s days after return" + ).format(trailDays); + } else if (leadDays > 0) { + message = + __("Selecting start date") + + ". " + + __( + "Lead period: %s days before start" + ).format(leadDays); + } else if (trailDays > 0) { + message = + __("Selecting start date") + + ". " + + __( + "Trail period: %s days after return" + ).format(trailDays); + } else { + message = __( + "Selecting start date" + ); + } + messageType = "info"; + } else { + if (trailDays > 0) { + message = + __("Selecting end date") + + ". " + + __( + "Trail period: %s days after return" + ).format(trailDays); + } else { + message = __("Selecting end date"); + } + messageType = "info"; + } + + // Show additional context if hovering over existing booking periods + if (isInExistingLead) { + message += + " • " + + __( + "hovering existing booking's lead period" + ); + } else if (isInExistingTrail) { + message += + " • " + + __( + "hovering existing booking's trail period" + ); + } + } + + feedbackDiv.textContent = message; + feedbackDiv.classList.remove( + "alert-danger", + "alert-warning", + "alert-info" + ); + + if (message) { + feedbackDiv.classList.remove("d-none"); + // Apply appropriate Bootstrap alert class based on message type + if (messageType === "error") { + feedbackDiv.classList.add( + "alert-danger" + ); + } else if (messageType === "warning") { + feedbackDiv.classList.add( + "alert-warning" + ); + } else { + feedbackDiv.classList.add("alert-info"); + } + } else { + feedbackDiv.classList.add("d-none"); + } + } } } ); -- 2.52.0